Bug: Fix incorrect deployment annotations #1662

awgreene · 2020-07-21T18:32:33Z

Problem: When OLM checks that the Operator's Deployment was created with the correct annotations, it will not update and redeploy the operator if the annotations are incorrect.

Cause: This issue was found when creating an Operator in an OperatorGroup that uses a LabelSelector to identify namespaces in the OperatorGroup. When the operator is created, the set of namespaces returned by the LabelSelector may be different from the set of namespaces when checking the deployment annotations.

Solution: OLM should rebuild the deployment with the correct set of annotations.

exdx · 2020-07-21T18:59:29Z

So the logic is if the annotation is missing/wrong OLM will update the annotation on the deployment or delete/recreate the deployment with the new annotation. by going into "InstallReady" are we going to just update the diff or actually recreate the deployment?
/lgtm

Bowenislandsong · 2020-07-21T19:12:25Z

This PR failed tests for 1 times with 1 individual failed tests and 4 skipped tests. A test is considered flaky if failed on multiple commits.

totaltestcount: 1
flaketestcount: 1
skippedtestcount: 4
flaketests:

classname: End-to-end
name: 'Operator when a subscription to a package exists should automatically
adopt components'
counts: 1
details:

count: 1
error: |4-

/home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/operator_test.go:218
Timed out after 60.213s.
Error: Unexpected non-nil/non-zero extra argument at index 1:
	<*errors.StatusError>: &errors.StatusError{ErrStatus:v1.Status{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ListMeta:v1.ListMeta{SelfLink:"", ResourceVersion:"", Continue:"", RemainingItemCount:(*int64)(nil)}, Status:"Failure", Message:"etcdserver: request timed out", Reason:"", Details:(*v1.StatusDetails)(nil), Code:500}}
/home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/operator_test.go:268

meandurationsec: 61.692282
skippedtests:

classname: End-to-end
name: 'Catalog image update'
counts: 1
details: []
meandurationsec: 0.217134
classname: End-to-end
name: 'Subscriptions create required objects from Catalogs Given a Namespace
when a CatalogSource is created with a bundle that contains prometheus objects
creating a subscription using the CatalogSource should install the operator
successfully'
counts: 1
details: []
meandurationsec: 2.29714
classname: End-to-end
name: 'Subscription updates existing install plan'
counts: 1
details: []
meandurationsec: 0.323676
classname: End-to-end
name: 'Subscriptions create required objects from Catalogs Given a Namespace
when a CatalogSource is created with a bundle that contains prometheus objects
creating a subscription using the CatalogSource should have created the expected
prometheus objects'
counts: 1
details: []
meandurationsec: 2.31187

awgreene · 2020-07-21T19:37:28Z

@exdx OLM is actually recreating the deployment, and then updates the deployment
.

ecordell · 2020-07-21T19:41:11Z

An e2e might be overkill, but is there an existing unit test we can add a case to?

Problem: When OLM checks that the Operator's Deployment was created with the correct annotations, it will not update and redeploy the operator if the annotations are incorrect. Cause: This issue was found when creating an Operator in an OperatorGroup that uses a LabelSelector to identify namespaces in the OperatorGroup. When the operator is created, the set of namespaces returned by the LabelSelector may be different from the set of namespaces when checking the deployment annotations. Solution: OLM should rebuild the deployment with the correct set of annotations.

awgreene · 2020-07-21T19:59:48Z

@ecordell added a unit test!

ecordell

/lgtm

openshift-ci-robot · 2020-07-21T20:48:43Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awgreene, ecordell

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:

~~OWNERS~~ [ecordell]

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

openshift-bot · 2020-07-21T21:53:47Z

/retest