Skip to content

Conversation

emy
Copy link
Contributor

@emy emy commented Jul 3, 2025

- What I did
While looking into OPNET-683 I ran into the issue that the config applied by MCO would not be copied over from /etc/nmstate/openshift to /etc/nmstate by nmstate-configuration.service. This caused manual intervention on the individual nodes to apply for example disruptive changes to the br-ex interface via the configs in /etc/nmstate/openshift. To get around the manual intervention I implemented a similar behavior to the one in /etc/nmstate. The config in /etc/nmstate/openshift will be compared to what is already applied in the /etc/nmstate/openshift/applied file. If the hashes mismatch there must be a config change and the config will be copied over to /etc/nmstate.

- How to verify it

  • Apply a MCO that changes the /etc/nmstate/openshift/[cluster.yml | hostname.yml].
  • Apply another MCO that triggers a reboot of the nodes.
  • Check if the /etc/nmstate/openshift/applied file contains the applied configs from Step 1.
  • Check if the changes also reflected on the network interfaces.

- Description for the changelog

Changes to /etc/nmstate/openshift/[cluster.yml | hostname.yml] will be applied on reboot now.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 3, 2025

@emy: This pull request references OPNET-683 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set.

In response to this:

- What I did
While looking into OPNET-683 I ran into the issue that the config applied by MCO would not be copied over from /etc/nmstate/openshift to /etc/nmstate by nmstate-configuration.service. This caused manual intervention on the individual nodes to apply for example disruptive changes to the br-ex interface via the configs in /etc/nmstate/openshift. To get around the manual intervention I implemented a similar behavior to the one in /etc/nmstate. The config in /etc/nmstate/openshift will be compared to what is already applied in the /etc/nmstate/openshift/applied file. If the hashes mismatch there must be a config change and the config will be copied over to /etc/nmstate.

- How to verify it

  • Apply a MCO that changes the /etc/nmstate/openshift/[cluster.yml | hostname.yml].
  • Apply another MCO that triggers a reboot of the nodes.
  • Check if the /etc/nmstate/openshift/applied file contains the applied configs from Step 1.
  • Check if the changes also reflected on the network interfaces.

- Description for the changelog

Changes to /etc/nmstate/openshift/[cluster.yml | hostname.yml] will be applied on reboot now.

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 3, 2025
@mkowalski
Copy link
Contributor

/cc @cybertron

@openshift-ci openshift-ci bot requested a review from cybertron July 18, 2025 11:36
@mkowalski
Copy link
Contributor

/lgtm

But @cybertron should review too. To me the code is reasonable but I don't know enough to judge if this ultimately fixes the issue

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 18, 2025
@cybertron
Copy link
Member

/lgtm

This may be somewhat stuck pending the e2e-gcp-op job getting fixed. It's been mostly failing lately, even on patches like this that don't affect it.

@cybertron
Copy link
Member

/retest-required

@cybertron
Copy link
Member

/assign @isabella-janssen

Looks like this is passing now. Should be good to go.

@mkowalski
Copy link
Contributor

/jira backport release-4.16,release-4.17,release-4.18,release-4.19

@openshift-ci-robot
Copy link
Contributor

@mkowalski: The following backport issues have been created:

Queuing cherrypicks to the requested branches to be created after this PR merges:
/cherrypick release-4.16
/cherrypick release-4.17
/cherrypick release-4.18
/cherrypick release-4.19

In response to this:

/jira backport release-4.16,release-4.17,release-4.18,release-4.19

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-cherrypick-robot

@openshift-ci-robot: once the present PR merges, I will cherry-pick it on top of release-4.16, release-4.17, release-4.18, release-4.19 in new PRs and assign them to you.

In response to this:

@mkowalski: The following backport issues have been created:

Queuing cherrypicks to the requested branches to be created after this PR merges:
/cherrypick release-4.16
/cherrypick release-4.17
/cherrypick release-4.18
/cherrypick release-4.19

In response to this:

/jira backport release-4.16,release-4.17,release-4.18,release-4.19

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 openshift-eng/jira-lifecycle-plugin repository.

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.

Copy link
Contributor

@yuqi-zhang yuqi-zhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logically seems fine to me, I guess the main behaviour change is that if something else is writing /etc/nmstate for some reason, this will overwrite it always, whereas previous it would be skipped?

Copy link
Contributor

openshift-ci bot commented Aug 22, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cybertron, emy, mkowalski, yuqi-zhang

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 Aug 22, 2025
@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD fa31b1c and 2 for PR HEAD ff1af76 in total

Copy link
Contributor

openshift-ci bot commented Aug 22, 2025

@emy: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-e2e-aws-ovn ff1af76 link false /test okd-scos-e2e-aws-ovn
ci/prow/e2e-gcp-op ff1af76 link true /test e2e-gcp-op
ci/prow/e2e-azure-ovn-upgrade-out-of-change ff1af76 link false /test e2e-azure-ovn-upgrade-out-of-change

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.

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD fa31b1c and 2 for PR HEAD ff1af76 in total

@openshift-merge-bot openshift-merge-bot bot merged commit cc2f713 into openshift:main Aug 22, 2025
19 of 22 checks passed
@openshift-cherrypick-robot

@openshift-ci-robot: #5162 failed to apply on top of branch "release-4.19":

Applying: Add improved check for already applied configs in nmstate-configuration.sh
.git/rebase-apply/patch:25: trailing whitespace.
    
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	templates/common/_base/files/nmstate-configuration.yaml
Falling back to patching base and 3-way merge...
Auto-merging templates/common/_base/files/nmstate-configuration.yaml
Applying: Removed override protection of /etc/nmstate/cluster.yml in nmstate-configuration.sh
Using index info to reconstruct a base tree...
M	templates/common/_base/files/nmstate-configuration.yaml
Falling back to patching base and 3-way merge...
Auto-merging templates/common/_base/files/nmstate-configuration.yaml
CONFLICT (content): Merge conflict in templates/common/_base/files/nmstate-configuration.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0002 Removed override protection of /etc/nmstate/cluster.yml in nmstate-configuration.sh

In response to this:

@mkowalski: The following backport issues have been created:

Queuing cherrypicks to the requested branches to be created after this PR merges:
/cherrypick release-4.16
/cherrypick release-4.17
/cherrypick release-4.18
/cherrypick release-4.19

In response to this:

/jira backport release-4.16,release-4.17,release-4.18,release-4.19

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 openshift-eng/jira-lifecycle-plugin repository.

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.

@openshift-cherrypick-robot

@openshift-ci-robot: new pull request created: #5262

In response to this:

@mkowalski: The following backport issues have been created:

Queuing cherrypicks to the requested branches to be created after this PR merges:
/cherrypick release-4.16
/cherrypick release-4.17
/cherrypick release-4.18
/cherrypick release-4.19

In response to this:

/jira backport release-4.16,release-4.17,release-4.18,release-4.19

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 openshift-eng/jira-lifecycle-plugin repository.

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.

@openshift-cherrypick-robot

@openshift-ci-robot: new pull request created: #5263

In response to this:

@mkowalski: The following backport issues have been created:

Queuing cherrypicks to the requested branches to be created after this PR merges:
/cherrypick release-4.16
/cherrypick release-4.17
/cherrypick release-4.18
/cherrypick release-4.19

In response to this:

/jira backport release-4.16,release-4.17,release-4.18,release-4.19

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 openshift-eng/jira-lifecycle-plugin repository.

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.

@openshift-cherrypick-robot

@openshift-ci-robot: #5162 failed to apply on top of branch "release-4.18":

Applying: Add improved check for already applied configs in nmstate-configuration.sh
.git/rebase-apply/patch:25: trailing whitespace.
    
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	templates/common/_base/files/nmstate-configuration.yaml
Falling back to patching base and 3-way merge...
Auto-merging templates/common/_base/files/nmstate-configuration.yaml
Applying: Removed override protection of /etc/nmstate/cluster.yml in nmstate-configuration.sh
Using index info to reconstruct a base tree...
M	templates/common/_base/files/nmstate-configuration.yaml
Falling back to patching base and 3-way merge...
Auto-merging templates/common/_base/files/nmstate-configuration.yaml
CONFLICT (content): Merge conflict in templates/common/_base/files/nmstate-configuration.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0002 Removed override protection of /etc/nmstate/cluster.yml in nmstate-configuration.sh

In response to this:

@mkowalski: The following backport issues have been created:

Queuing cherrypicks to the requested branches to be created after this PR merges:
/cherrypick release-4.16
/cherrypick release-4.17
/cherrypick release-4.18
/cherrypick release-4.19

In response to this:

/jira backport release-4.16,release-4.17,release-4.18,release-4.19

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 openshift-eng/jira-lifecycle-plugin repository.

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.

@yuqi-zhang
Copy link
Contributor

Hmm, this seems to have failed to cherry-pick to 4.19/4.18 but was successful for 4.17 and 4.16. Hopefully those are correct?

Since each backport requires the previous version, it's best to cherry pick one branch at a time, instead of mass cherrypicking, since errors like this can occur.

@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

Distgit: ose-machine-config-operator
This PR has been included in build ose-machine-config-operator-container-v4.21.0-202508230015.p0.gcc2f713.assembly.stream.el9.
All builds following this will include this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants