OCPBUGS-6859: Create empty manifest directory when installing MicroShift by ggiguash · Pull Request #1373 · openshift/microshift

ggiguash · 2023-02-14T19:52:44Z

The /etc/microshift/manifests empty directory is created when installing RPMs

Closes OCPBUGS-6859

openshift-ci-robot · 2023-02-14T19:52:52Z

@ggiguash: This pull request references Jira Issue OCPBUGS-6859, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.13.0) matches configured target version for branch (4.13.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

MicroShift attempts to load manifest files from /etc/microshift/manifests and /usr/etc/microshift/manifests directories

The /etc/microshift/manifests empty directory is created when installing RPMs

Clean up unused directories
/var/hpvolumes; \
/var/run/kubelet; \
/var/lib/kubelet/pods; \
/var/run/secrets/kubernetes.io/serviceaccount
Closes OCPBUGS-6859

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-02-14T20:51:25Z

@ggiguash: This pull request references Jira Issue OCPBUGS-6859, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.13.0) matches configured target version for branch (4.13.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

MicroShift attempts to load manifest files from /etc/microshift/manifests and /usr/etc/microshift/manifests directories

The /etc/microshift/manifests empty directory is created when installing RPMs

Clean up unused directories
/var/hpvolumes
/var/run/kubelet
/var/lib/kubelet/pods
/var/run/secrets/kubernetes.io/serviceaccount
Closes OCPBUGS-6859

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

pmtk · 2023-02-15T13:56:15Z

/lgtm

ggiguash · 2023-02-15T14:23:22Z

@pmtk , I missed a few places to replace the manifests directory. More files added now - can you look at it again?

dhellmann

I don't know enough about the FHS standard to know why the original path may have been selected. @fzdarsky can you comment?

openshift-ci-robot · 2023-02-16T15:49:28Z

@ggiguash: This pull request references Jira Issue OCPBUGS-6859, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.13.0) matches configured target version for branch (4.13.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

MicroShift attempts to load manifest files from /etc/microshift/manifests and /usr/etc/microshift/manifests directories

The /etc/microshift/manifests empty directory is created when installing RPMs

Closes OCPBUGS-6859

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ggiguash · 2023-02-16T16:21:11Z

/retest

dhellmann · 2023-02-16T19:12:03Z

This is going to trigger documentation updates, too.

What is the motivation for changing /usr/lib/ to /usr/etc/? How are those directories expected to be used on an ostree-based system?

/hold

ggiguash · 2023-02-17T06:39:06Z

What is the motivation for changing /usr/lib/ to /usr/etc/? How are those directories expected to be used on an ostree-based system?

The reason is due to symmetry with config.yaml file, which is located in the /etc or /usr/etc pair.
So, the situation I was trying to fix is the following

/etc/microshift/config.yaml.default -> /usr/etc/
/etc/microshift/mamifests -> /usr/lib/microshift/manifests
We had 2 configuration directories under /usr.

dhellmann · 2023-02-17T20:08:57Z

What is the motivation for changing /usr/lib/ to /usr/etc/? How are those directories expected to be used on an ostree-based system?

The reason is due to symmetry with config.yaml file, which is located in the /etc or /usr/etc pair. So, the situation I was trying to fix is the following

/etc/microshift/config.yaml.default -> /usr/etc/

/etc/microshift/mamifests -> /usr/lib/microshift/manifests
We had 2 configuration directories under /usr.

These paths have meanings, though, right? Each has a purpose and users are expected to put different types of content in each of them? How does https://refspecs.linuxfoundation.org/FHS_3.0/fhs/index.html map to the parts of the filesystem that are readonly in ostree images?

openshift-ci-robot · 2023-02-20T17:48:28Z

@ggiguash: This pull request references Jira Issue OCPBUGS-6859, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.13.0) matches configured target version for branch (4.13.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

The /etc/microshift/manifests empty directory is created when installing RPMs

Closes OCPBUGS-6859

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ggiguash · 2023-02-20T17:55:08Z

@dhellmann, I reverted the manifest directory change, as discussed.
/unhold

dhellmann

One open question but we can deal with it separately

/lgtm

dhellmann · 2023-02-20T18:21:33Z

packaging/rpm/microshift.spec

 install -p -m644 packaging/systemd/microshift.service %{buildroot}%{_unitdir}/microshift.service

 install -d -m755 %{buildroot}/%{_sysconfdir}/microshift
+install -d -m755 %{buildroot}/%{_sysconfdir}/microshift/manifests


Do we need /usr/lib/microshift and /usr/lib/microshift/manifests, too? Maybe we should add those in another PR, in case we change the names?

The /usr/lib/microshift directory is not created at present.
Since we're talking about read-only directory structure that can only be updated from RPM, I thought it does not make sense to create an empty hierarchy.

OK, I thought we might need to declare that we own that directory so that other RPMs can just put files into it.

openshift-ci · 2023-02-20T18:23:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhellmann, ggiguash, pmtk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [dhellmann,ggiguash,pmtk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2023-02-20T19:43:46Z

/retest-required

Remaining retests: 0 against base HEAD 94cdb19 and 2 for PR HEAD 08df195 in total

openshift-ci · 2023-02-20T20:21:10Z

@ggiguash: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot · 2023-02-20T20:25:20Z

@ggiguash: All pull requests linked via external trackers have merged:

openshift/microshift#1373

Jira Issue OCPBUGS-6859 has been moved to the MODIFIED state.

Details

In response to this:

The /etc/microshift/manifests empty directory is created when installing RPMs

Closes OCPBUGS-6859

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci bot requested review from copejon and sallyom February 14, 2023 19:53

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2023

ggiguash force-pushed the manifest_folder branch from f260b1e to 898bd00 Compare February 14, 2023 19:56

openshift-ci bot assigned pmtk Feb 15, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 15, 2023

ggiguash force-pushed the manifest_folder branch from 898bd00 to 7112720 Compare February 15, 2023 14:18

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 15, 2023

ggiguash force-pushed the manifest_folder branch from 7112720 to bdc8fbc Compare February 15, 2023 14:22

ggiguash force-pushed the manifest_folder branch from bdc8fbc to de27527 Compare February 15, 2023 14:29

dhellmann reviewed Feb 15, 2023

View reviewed changes

ggiguash force-pushed the manifest_folder branch 3 times, most recently from c773465 to 1f7d00c Compare February 16, 2023 15:48

ggiguash changed the title ~~OCPBUGS-6859: Consolidate manifest directory location and clean up RPM spec file~~ OCPBUGS-6859: Consolidate manifest directory location Feb 16, 2023

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 16, 2023

ggiguash changed the title ~~OCPBUGS-6859: Consolidate manifest directory location~~ OCPBUGS-6859: Consolidate manifest directory location and creation Feb 17, 2023

ggiguash force-pushed the manifest_folder branch from 1f7d00c to 70bab61 Compare February 17, 2023 07:20

ggiguash force-pushed the manifest_folder branch from 70bab61 to 2136a63 Compare February 20, 2023 17:43

ggiguash changed the title ~~OCPBUGS-6859: Consolidate manifest directory location and creation~~ OCPBUGS-6859: Create empty manifest directory when installing MicroShift Feb 20, 2023

Create empty manifests directory

08df195

ggiguash force-pushed the manifest_folder branch from 2136a63 to 08df195 Compare February 20, 2023 17:54

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 20, 2023

dhellmann reviewed Feb 20, 2023

View reviewed changes

openshift-ci bot assigned dhellmann Feb 20, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 20, 2023

openshift-merge-robot merged commit 172d1d7 into openshift:main Feb 20, 2023

ggiguash deleted the manifest_folder branch February 22, 2023 13:17

microshift-rebase-script bot mentioned this pull request Jul 26, 2024

NO-ISSUE: rebase-main-4.17.0-0.nightly-2024-07-25-142842_amd64-2024-07-25_arm64-2024-07-25 #3683

Merged

Conversation

ggiguash commented Feb 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Feb 14, 2023

Uh oh!

openshift-ci-robot commented Feb 14, 2023

Uh oh!

pmtk commented Feb 15, 2023

Uh oh!

ggiguash commented Feb 15, 2023

Uh oh!

dhellmann left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Feb 16, 2023

Uh oh!

ggiguash commented Feb 16, 2023

Uh oh!

dhellmann commented Feb 16, 2023

Uh oh!

ggiguash commented Feb 17, 2023

Uh oh!

dhellmann commented Feb 17, 2023

Uh oh!

openshift-ci-robot commented Feb 20, 2023

Uh oh!

ggiguash commented Feb 20, 2023

Uh oh!

dhellmann left a comment

Choose a reason for hiding this comment

Uh oh!

dhellmann Feb 20, 2023

Choose a reason for hiding this comment

Uh oh!

ggiguash Feb 20, 2023

Choose a reason for hiding this comment

Uh oh!

dhellmann Feb 20, 2023

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Feb 20, 2023

Uh oh!

openshift-ci-robot commented Feb 20, 2023

Uh oh!

openshift-ci bot commented Feb 20, 2023

Uh oh!

openshift-ci-robot commented Feb 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ggiguash commented Feb 14, 2023 •

edited

Loading