-
Notifications
You must be signed in to change notification settings - Fork 27
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.
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
OCPBUGS-37300: Fix forced reinit issue #619
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,22 +125,41 @@ func HasReinitAnnotation(fi *v1alpha1.FileIntegrity) (nodes []string, annotation | |
// changed. | ||
func GetAddedNodeHoldoffAnnotation(fi *v1alpha1.FileIntegrity, nodeName string) (map[string]string, bool) { | ||
ficopy := fi.DeepCopy() | ||
if fi.Annotations == nil { | ||
if ficopy.Annotations == nil { | ||
ficopy.Annotations = make(map[string]string) | ||
} | ||
|
||
if nodeList, has := fi.Annotations[IntegrityHoldoffAnnotationKey]; has { | ||
if nodeList == "" { | ||
// no need to add the node if all nodes are in holdoff | ||
return ficopy.Annotations, false | ||
} | ||
if strings.Contains(nodeList, nodeName) { | ||
nodeList, has := fi.Annotations[IntegrityHoldoffAnnotationKey] | ||
if !has { | ||
// If the annotation key doesn't exist, simply create it with this nodeName | ||
ficopy.Annotations[IntegrityHoldoffAnnotationKey] = nodeName | ||
return ficopy.Annotations, true | ||
} | ||
|
||
AddedNode := strings.TrimSpace(nodeName) | ||
|
||
// If nodeList is an empty string, the current logic indicates | ||
if nodeList == "" { | ||
// "no need to add the node if all nodes are in holdoff". | ||
return ficopy.Annotations, false | ||
} | ||
|
||
// Split existing nodeList into slice | ||
nodes := strings.Split(nodeList, ",") | ||
|
||
// Check if nodeName is already in the list | ||
for _, n := range nodes { | ||
if strings.EqualFold(strings.TrimSpace(n), AddedNode) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the main issue was because https://github.com/openshift/file-integrity-operator/pull/619/files#diff-a3206732c69662d901d8e5ccf08d084332c8dbfb58c67996b5bb9ec5b6ed4425L164, the replace is not correct |
||
// Node is already in holdoff, so no changes | ||
return ficopy.Annotations, false | ||
} | ||
ficopy.Annotations[IntegrityHoldoffAnnotationKey] = nodeList + "," + nodeName | ||
} else { | ||
ficopy.Annotations[IntegrityHoldoffAnnotationKey] = nodeName | ||
} | ||
|
||
// Not found, append it | ||
nodes = append(nodes, AddedNode) | ||
// Update the annotation with the new list | ||
ficopy.Annotations[IntegrityHoldoffAnnotationKey] = strings.Join(nodes, ",") | ||
|
||
return ficopy.Annotations, true | ||
} | ||
|
||
|
@@ -151,22 +170,39 @@ func GetRemovedNodeHoldoffAnnotation(fi *v1alpha1.FileIntegrity, nodeName string | |
if !IsNodeInHoldoff(fi, nodeName) { | ||
return nil, false | ||
} | ||
|
||
ficopy := fi.DeepCopy() | ||
if fi.Annotations == nil { | ||
if ficopy.Annotations == nil { | ||
ficopy.Annotations = make(map[string]string) | ||
} | ||
if nodeList, has := fi.Annotations[IntegrityHoldoffAnnotationKey]; has { | ||
if nodeList == nodeName { | ||
// remove the annotation if all nodes are in holdoff or if the node is the only one in holdoff | ||
delete(ficopy.Annotations, IntegrityHoldoffAnnotationKey) | ||
} else { | ||
// remove the node from the holdoff list string and update the annotation along with comma separators | ||
nodeList = strings.Replace(nodeList, ","+nodeName, "", -1) | ||
ficopy.Annotations[IntegrityHoldoffAnnotationKey] = nodeList | ||
|
||
nodeList, has := ficopy.Annotations[IntegrityHoldoffAnnotationKey] | ||
if !has { | ||
return nil, false | ||
} | ||
removedNode := strings.TrimSpace(nodeName) | ||
|
||
// Split the node list on commas | ||
nodes := strings.Split(nodeList, ",") | ||
var newNodes []string | ||
|
||
// Filter out the nodeName | ||
for _, n := range nodes { | ||
existingNode := strings.TrimSpace(n) | ||
if !strings.EqualFold(existingNode, removedNode) { | ||
newNodes = append(newNodes, existingNode) | ||
} | ||
return ficopy.Annotations, true | ||
} | ||
return nil, false | ||
|
||
// If there are no nodes left in holdoff, remove the annotation entirely | ||
if len(newNodes) == 0 { | ||
delete(ficopy.Annotations, IntegrityHoldoffAnnotationKey) | ||
} else { | ||
// Otherwise, join the remaining nodes and update the annotation | ||
ficopy.Annotations[IntegrityHoldoffAnnotationKey] = strings.Join(newNodes, ",") | ||
} | ||
|
||
return ficopy.Annotations, true | ||
} | ||
|
||
// GetAddedNodeReinitAnnotation returns the annotation value for the added node | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: We could adjust the visibility of this since it's only used in this function.