OCPBUGS-37300: Fix forced reinit issue #619

Vincent056 · 2025-01-21T11:14:18Z

This commit resolves issues in GetRemovedNodeHoldoffAnnotation and GetAddedNodeHoldoffAnnotation related to managing the holdoff annotation for FileIntegrity Object. Previously, node removal could fail in cases where the node to be removed was at the beginning, middle, or end of a comma separated list due to incorrect string manipulation.

This commit resolves issues in GetRemovedNodeHoldoffAnnotation and GetAddedNodeHoldoffAnnotation related to managing the holdoff annotation for FileIntegrity nodes. Previously, node removal could fail in cases where the node to be removed was at the beginning, middle, or end of a comma separated list due to incorrect string manipulation.

openshift-ci-robot · 2025-01-21T11:14:24Z

@Vincent056: This pull request references Jira Issue OCPBUGS-37300, which is invalid:

expected the bug to target the "4.19.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

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

Details

In response to this:

This commit resolves issues in GetRemovedNodeHoldoffAnnotation and GetAddedNodeHoldoffAnnotation related to managing the holdoff annotation for FileIntegrity Object. Previously, node removal could fail in cases where the node to be removed was at the beginning, middle, or end of a comma separated list due to incorrect string manipulation.

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 · 2025-01-21T11:15:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Vincent056

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~~ [Vincent056]

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

openshift-ci · 2025-01-21T14:10:40Z

@Vincent056: The following test 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/e2e-rosa	`b386997`	link	false	`/test e2e-rosa`

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

rhmdnd

Looks like a reasonable change. Is there a way to force the operator into a state where it needs to exercise this code in an e2e test?

rhmdnd · 2025-02-05T21:49:54Z

pkg/common/util.go

+		return ficopy.Annotations, true
+	}
+
+	AddedNode := strings.TrimSpace(nodeName)


nit: We could adjust the visibility of this since it's only used in this function.

rhmdnd · 2025-02-05T22:53:01Z

pkg/common/util.go

+
+	// Check if nodeName is already in the list
+	for _, n := range nodes {
+		if strings.EqualFold(strings.TrimSpace(n), AddedNode) {


Is this where the main issue was? Otherwise, most of the logic looks the same, just in a different order, but here we changed the comparison from using strings.Contains to iterating the entire list of nodes and comparing them.

the main issue was because https://github.com/openshift/file-integrity-operator/pull/619/files#diff-a3206732c69662d901d8e5ccf08d084332c8dbfb58c67996b5bb9ec5b6ed4425L164, the replace is not correct

xiaojiey · 2025-02-08T03:34:06Z

@Vincent056 I deployed the fio operator from this PR #619. Two test scenarios tested. Both passed.
Scenario 1: Add annotations manually
I added the annotation file-integrity.openshift.io/holdoff:ip-10-0-50-40.us-east-2.compute.internal,ip-10-0-74-36.us-east-2.compute.internal,node-not-exist for fileintegrity manually. The annotations for the two nodes in ready status will be cleared in about 2-3 minutes.

% oc get fileintegrity example-fileintegrity -o custom-columns=NAME:metadata.name,ANNOTATIONS:metadata.annotations -n openshift-file-integrity -w
NAME                    ANNOTATIONS
example-fileintegrity   map[file-integrity.openshift.io/holdoff:ip-10-0-50-40.us-east-2.compute.internal,ip-10-0-74-36.us-east-2.compute.internal,node-not-exist kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}
]
example-fileintegrity   map[file-integrity.openshift.io/holdoff:ip-10-0-50-40.us-east-2.compute.internal,node-not-exist file-integrity.openshift.io/re-init:ip-10-0-74-36.us-east-2.compute.internal kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}
]
example-fileintegrity   map[file-integrity.openshift.io/holdoff:ip-10-0-50-40.us-east-2.compute.internal,node-not-exist file-integrity.openshift.io/re-init:ip-10-0-74-36.us-east-2.compute.internal kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}
]
example-fileintegrity   map[file-integrity.openshift.io/holdoff:ip-10-0-50-40.us-east-2.compute.internal,node-not-exist kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}
]
example-fileintegrity   map[file-integrity.openshift.io/holdoff:ip-10-0-50-40.us-east-2.compute.internal,node-not-exist kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}
]
example-fileintegrity   map[file-integrity.openshift.io/holdoff:node-not-exist file-integrity.openshift.io/re-init:ip-10-0-50-40.us-east-2.compute.internal kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}

Scenario 2:

Scale up machineset add add more workers.
oc patch mcp worker --type='merge' -p '{"spec": {"maxUnavailable": 2}}'
Create a mc to trigger worker mcp restart.
Watch the annotations on fileintegrity:

% oc get fileintegrity example-fileintegrity -o custom-columns=NAME:metadata.name,ANNOTATIONS:metadata.annotations -n openshift-file-integrity -w
NAME                    ANNOTATIONS
example-fileintegrity   map[file-integrity.openshift.io/holdoff:node-not-exist,ip-10-0-1-108.us-east-2.compute.internal,ip-10-0-50-40.us-east-2.compute.internal kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}
]
example-fileintegrity   map[file-integrity.openshift.io/holdoff:node-not-exist,ip-10-0-1-108.us-east-2.compute.internal,ip-10-0-50-40.us-east-2.compute.internal kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}
]
example-fileintegrity   map[file-integrity.openshift.io/holdoff:node-not-exist,ip-10-0-1-108.us-east-2.compute.internal,ip-10-0-50-40.us-east-2.compute.internal kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}
]
example-fileintegrity   map[file-integrity.openshift.io/holdoff:node-not-exist,ip-10-0-50-40.us-east-2.compute.internal file-integrity.openshift.io/re-init:ip-10-0-1-108.us-east-2.compute.internal kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}
...
% oc get fileintegrity example-fileintegrity -o custom-columns=NAME:metadata.name,ANNOTATIONS:metadata.annotations -n openshift-file-integrity -w
NAME                    ANNOTATIONS
example-fileintegrity   map[file-integrity.openshift.io/holdoff:node-not-exist kubectl.kubernetes.io/last-applied-configuration:{"apiVersion":"fileintegrity.openshift.io/v1alpha1","kind":"FileIntegrity","metadata":{"annotations":{},"name":"example-fileintegrity","namespace":"openshift-file-integrity"},"spec":{"config":{"gracePeriod":60,"maxBackups":5},"debug":true}}
% oc get fileintegrity example-fileintegrity -o=jsonpath={.status.phase}
Active%

xiaojiey · 2025-02-08T03:40:23Z

/label qe-approved

xiaojiey · 2025-02-08T03:41:14Z

/jira refresh

openshift-ci-robot · 2025-02-08T03:41:18Z

@xiaojiey: This pull request references Jira Issue OCPBUGS-37300, which is invalid:

expected the bug to target the "4.19.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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.

xiaojiey · 2025-02-08T03:42:34Z

/jira refresh

openshift-ci-robot · 2025-02-08T03:42:40Z

@xiaojiey: This pull request references Jira Issue OCPBUGS-37300, 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.19.0) matches configured target version for branch (4.19.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @xiaojiey

Details

In response to this:

/jira refresh

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.

yuumasato

/lgtm

@Vincent056 Would be good to add tests, either unit or e2e.

openshift-ci-robot · 2025-03-13T17:38:21Z

@Vincent056: Jira Issue OCPBUGS-37300: All pull requests linked via external trackers have merged:

openshift/file-integrity-operator#619

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

Details

In response to this:

This commit resolves issues in GetRemovedNodeHoldoffAnnotation and GetAddedNodeHoldoffAnnotation related to managing the holdoff annotation for FileIntegrity Object. Previously, node removal could fail in cases where the node to be removed was at the beginning, middle, or end of a comma separated list due to incorrect string manipulation.

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.

OCPBUGS-37300: Fix forced reinit issue

openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Jan 21, 2025

openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Jan 21, 2025

openshift-ci bot requested review from xiaojiey and yuumasato January 21, 2025 11:15

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 21, 2025

rhmdnd reviewed Feb 5, 2025

View reviewed changes

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Feb 8, 2025

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Feb 8, 2025

openshift-ci-robot removed the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Feb 8, 2025

yuumasato reviewed Mar 13, 2025

View reviewed changes

openshift-ci bot assigned yuumasato Mar 13, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 13, 2025

openshift-merge-bot bot merged commit dfa00b9 into master Mar 13, 2025
12 of 13 checks passed

Vincent056 pushed a commit that referenced this pull request Nov 20, 2025

Merge pull request #619 from openshift/OCPBUGS-37300

ea39ca9

OCPBUGS-37300: Fix forced reinit issue

Vincent056 pushed a commit that referenced this pull request Nov 20, 2025

Merge pull request #619 from openshift/OCPBUGS-37300

348a10e

OCPBUGS-37300: Fix forced reinit issue

OCPBUGS-37300: Fix forced reinit issue #619

OCPBUGS-37300: Fix forced reinit issue #619

Uh oh!

Conversation

Vincent056 commented Jan 21, 2025

Uh oh!

openshift-ci-robot commented Jan 21, 2025

Uh oh!

openshift-ci bot commented Jan 21, 2025

Uh oh!

openshift-ci bot commented Jan 21, 2025

Uh oh!

rhmdnd left a comment

Choose a reason for hiding this comment

Uh oh!

rhmdnd Feb 5, 2025

Choose a reason for hiding this comment

Uh oh!

rhmdnd Feb 5, 2025

Choose a reason for hiding this comment

Uh oh!

Vincent056 Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

xiaojiey commented Feb 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xiaojiey commented Feb 8, 2025

Uh oh!

xiaojiey commented Feb 8, 2025

Uh oh!

openshift-ci-robot commented Feb 8, 2025

Uh oh!

xiaojiey commented Feb 8, 2025

Uh oh!

openshift-ci-robot commented Feb 8, 2025

Uh oh!

yuumasato left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openshift-ci-robot commented Mar 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xiaojiey commented Feb 8, 2025 •

edited

Loading