Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

manifests: Clean-up NVME-related IDs #1520

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

manifests: Clean-up NVME-related IDs #1520

wants to merge 1 commit into from

Conversation

travier
Copy link
Member

@travier travier commented May 29, 2024

See: #1519

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented May 29, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: travier

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 29, 2024
@travier
Copy link
Member Author

travier commented May 29, 2024

This will likely need more testing and investigation but let's see if this breaks immediately or if we can invest in testing.

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented May 29, 2024

@travier: all tests passed!

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

@jlebon
Copy link
Member

jlebon commented May 29, 2024

Didn't dig into this but two things offhand:

  1. What e.g. scriptlet exactly creates these? Maybe there's a cleaner way to do this, including working with the package maintainers directly.
  2. We need to ensure that the identifiers are correctly generated on first boot (or whenever they're necessary).

@travier
Copy link
Member Author

travier commented May 29, 2024

100% agree.
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 29, 2024
@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 28, 2024
@travier
Copy link
Member Author

travier commented Aug 28, 2024

/remove-lifecycle stale
/lifecycle frozen

@openshift-ci openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 28, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Aug 28, 2024

@travier: The lifecycle/frozen label cannot be applied to Pull Requests.

In response to this:

/remove-lifecycle stale
/lifecycle frozen

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-sigs/prow repository.

@cgwalters
Copy link
Member

  • We should have a tracker bug against the package
  • Workarounds like this ideally land in the bootc base image and are inherited from there

@jlebon
Copy link
Member

jlebon commented Sep 20, 2024

  • Workarounds like this ideally land in the bootc base image and are inherited from there

Yup, we're working up to that. Note nvme-cli isn't in tier-x, but the postprocess script would be harmless even if it doesn't exist.

(But again, this needs further investigation to make sure this is the right workaround.)

Akrog added a commit to Akrog/cinder-operator that referenced this pull request Sep 25, 2024
@Akrog
NVMe sample: Fix nvme ids
82a8c7e 
OpenShift image has the NVMe `hostid` and `hostnqn` hardcoded, so it
ends up being the same in all the control plane nodes, which is
problematic.

This `MachineConfig` fixes this issue by recreating both files when the
`hostid` doesn't match the system-uuid of the machine it is running on.

Bugs:
- https://issues.redhat.com/browse/OCPBUGS-34629
- openshift/os#1519
- openshift/os#1520
@Akrog Akrog mentioned this pull request Sep 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dustymabe dustymabe Awaiting requested review from dustymabe

@lukewarmtemp lukewarmtemp Awaiting requested review from lukewarmtemp

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@travier @jlebon @openshift-bot @cgwalters