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

Bug 1914932: Put correct resource name in relatedObjects #945

Merged
merged 1 commit into from
Jan 15, 2021
Merged

Bug 1914932: Put correct resource name in relatedObjects #945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion pkg/controller/operconfig/operconfig_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (r *ReconcileOperConfig) Reconcile(request reconcile.Request) (reconcile.Re
// Add operator.openshift.io/v1/network to relatedObjects for must-gather
relatedObjects = append(relatedObjects, configv1.ObjectReference{
Group: "operator.openshift.io",
Resource: "network",
Resource: "networks",
Name: "cluster",
})

Expand Down
7 changes: 5 additions & 2 deletions pkg/controller/statusmanager/status_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,10 @@ func (status *StatusManager) deleteRelatedObjectsNotRendered(co *configv1.Cluste
if status.relatedObjects == nil {
return
}

for _, currentObj := range co.Status.RelatedObjects {
var found bool = false
for _, renderedObj := range status.relatedObjects {
found = reflect.DeepEqual(currentObj, renderedObj)

if found {
break
}
Expand All @@ -97,6 +95,11 @@ func (status *StatusManager) deleteRelatedObjectsNotRendered(co *configv1.Cluste
log.Printf("Object Kind is Namespace, skip")
continue
}
// @aconstan: remove this after having the PR implementing this change, integrated.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ugh. If we add new objects to relatedObjects to get them must-gathered, then that means we'll end up deleting those objects when you downgrade to a version of CNO where they weren't part of relatedObjects. This may be the cause of the "openshift-sdn CRDs get deleted when downgrading" bug? (Not sure where that is or who was working on it...)

We need to avoid doing deleteRelatedObjectsNotRendered on downgrade. @squeed

For now, the fix is probably to backport this hack to 4.6 and 4.5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is the cause of it, @juanluisvaladas verified it with PR: #948. We need this back-ported.

if gvk.Kind == "Network" && gvk.Group == "operator.openshift.io" {
log.Printf("Object Kind is network.operator.openshift.io, skip")
continue
}
log.Printf("Detected related object with GVK %+v, namespace %v and name %v not rendered by manifests, deleting...", gvk, currentObj.Namespace, currentObj.Name)
objToDelete := &uns.Unstructured{}
objToDelete.SetName(currentObj.Name)
Expand Down