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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 57 additions & 21 deletions pkg/common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

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.


// 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) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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 strings.Contains to iterating the entire list of nodes and comparing them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

// 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
}

Expand All @@ -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
Expand Down