Bug 1991662: Catalog switcher by kevinrizza · Pull Request #144 · openshift/operator-framework-olm

kevinrizza · 2021-07-30T17:56:59Z

Implement catalog switching proposal

implements catalog switching proposal
See https://github.com/operator-framework/enhancements/blob/master/enhancements/automatic-catalog-switching-annotations.md
picks up v0.10.3
change sed command in copy_crds.sh to be posix compliant

The command + in sed is non-portable. The posix compliant version of
this command is {1,}

This change allows this script to run on a mac too (which uses BSD sed)

See https://riptutorial.com/sed/topic/9436/bsd-macos-sed-vs--gnu-sed-vs--the-posix-sed-specification

Note: GVK was implemented but disabled due to security concerns.
Future versions will address these concerns

Signed-off-by: John Hunkins jhunkins@us.ibm.com

Address review comments from ecordell

reorganize imports
change logging common case to debug

Signed-off-by: John Hunkins jhunkins@us.ibm.com

removed all watcher related code

this is the commit that should be reviewed later
when it comes time to re-enable the GVK code

Signed-off-by: John Hunkins jhunkins@us.ibm.com

update vendor to remove dynamic lister code

Signed-off-by: John Hunkins jhunkins@us.ibm.com

fix incorrect names and improper error usage

fix names that don't match Golang convention
fix misspelled package name
return nil for sync function casting

Signed-off-by: John Hunkins jhunkins@us.ibm.com

change code block organization and add mutex

Signed-off-by: John Hunkins jhunkins@us.ibm.com

remove all traces of GVK template feature

clean up every instance of the GVK templating feature
add last minute change for RWMutex instead of Mutex
this is the second commit that should be reviewed later
when it comes time to re-enable the GVK code

Signed-off-by: John Hunkins jhunkins@us.ibm.com

return status update errors and adjust logging

Signed-off-by: John Hunkins jhunkins@us.ibm.com

address review comments

update logging for clarity
clean up code blocks
refactor catalogsource helper functions:
- remove mutex
- remove two step get/update* in preference to update* even though
  this may result in more "the object has been modified" errors
- rename helper functions to clarify usage
- clarify the update helper function docs

Signed-off-by: John Hunkins jhunkins@us.ibm.com

Upstream-repository: operator-lifecycle-manager
Upstream-commit: 5f6fcd14f08a7e2a81b016fa83132ea68242f335

* Implement catalog switching proposal - implements catalog switching proposal See https://github.com/operator-framework/enhancements/blob/master/enhancements/automatic-catalog-switching-annotations.md - picks up v0.10.3 - change sed command in copy_crds.sh to be posix compliant The command + in sed is non-portable. The posix compliant version of this command is {1,} This change allows this script to run on a mac too (which uses BSD sed) See https://riptutorial.com/sed/topic/9436/bsd-macos-sed-vs--gnu-sed-vs--the-posix-sed-specification Note: GVK was implemented but disabled due to security concerns. Future versions will address these concerns Signed-off-by: John Hunkins <jhunkins@us.ibm.com> * Address review comments from ecordell - reorganize imports - change logging common case to debug Signed-off-by: John Hunkins <jhunkins@us.ibm.com> * removed all watcher related code - this is the commit that should be reviewed later when it comes time to re-enable the GVK code Signed-off-by: John Hunkins <jhunkins@us.ibm.com> * update vendor to remove dynamic lister code Signed-off-by: John Hunkins <jhunkins@us.ibm.com> * fix incorrect names and improper error usage - fix names that don't match Golang convention - fix misspelled package name - return nil for sync function casting Signed-off-by: John Hunkins <jhunkins@us.ibm.com> * change code block organization and add mutex Signed-off-by: John Hunkins <jhunkins@us.ibm.com> * remove all traces of GVK template feature - clean up every instance of the GVK templating feature - add last minute change for RWMutex instead of Mutex - this is the second commit that should be reviewed later when it comes time to re-enable the GVK code Signed-off-by: John Hunkins <jhunkins@us.ibm.com> * return status update errors and adjust logging Signed-off-by: John Hunkins <jhunkins@us.ibm.com> * address review comments - update logging for clarity - clean up code blocks - refactor catalogsource helper functions: - remove mutex - remove two step get/update* in preference to update* even though this may result in more "the object has been modified" errors - rename helper functions to clarify usage - clarify the update helper function docs Signed-off-by: John Hunkins <jhunkins@us.ibm.com> Upstream-repository: operator-lifecycle-manager Upstream-commit: 5f6fcd14f08a7e2a81b016fa83132ea68242f335

timflannagan · 2021-07-30T20:06:12Z

/retest

timflannagan · 2021-07-30T20:10:00Z

This lgtm to me but we're waiting for either exception approval or this to land as a bug in z-stream.

/hold

timflannagan · 2021-07-30T20:37:21Z

/retest

timflannagan · 2021-07-30T20:40:14Z

/hold cancel

davegord · 2021-07-30T20:44:26Z

/retest

timflannagan · 2021-07-30T23:31:52Z

/retest

davegord · 2021-07-31T02:40:32Z

/retest

davegord · 2021-07-31T12:17:35Z

/retest

kevinrizza · 2021-07-31T14:53:02Z

 Status: "Failure",
            Message: "Operation cannot be fulfilled on catalogsources.operators.coreos.com \"catalog-5dh25\": the object has been modified; please apply your changes to the latest version and try again",
            Reason: "Conflict",
            Details: {
                Name: "catalog-5dh25",
                Group: "operators.coreos.com",
                Kind: "catalogsources",
                UID: "",
                Causes: nil,
                RetryAfterSeconds: 0,
            },
            Code: 409,

Looks like we're not resilient to cache invalidation and need to add retry logic. I believe these failures are valid

/hold

so we don't keep wasting test resources

Upstream-repository: operator-lifecycle-manager Upstream-commit: d55593954671505ef56bf09eb2e7900069fedecb

kevinrizza · 2021-08-02T16:35:58Z

/hold cancel

tests updated, should avoid that flake now

In some cases, the discovery client can return a kube server version with the minor version field as a non uint value. To properly parse the value and get all version fields correctly, instead directly parse the GitVersion string for all values of the kube server version Upstream-repository: operator-lifecycle-manager Upstream-commit: 31350db3ec552fbabbedda33929538983155dea9

kevinrizza · 2021-08-04T15:19:19Z

/test e2e-gcp

kevinrizza · 2021-08-04T16:24:19Z

/retest

kevinrizza · 2021-08-04T17:32:01Z

/test e2e-gcp

kevinrizza · 2021-08-04T18:57:05Z

/test e2e-upgrade

davegord · 2021-08-04T19:43:35Z

/test e2e-gcp

kevinrizza · 2021-08-05T12:55:34Z

/retest

kevinrizza · 2021-08-05T15:34:39Z

/retest

kevinrizza · 2021-08-05T16:45:52Z

/test e2e-upgrade

kevinrizza · 2021-08-05T18:38:41Z

/test e2e-upgrade

kevinrizza · 2021-08-05T19:35:39Z

/test e2e-upgrade

kevinrizza · 2021-08-06T01:50:06Z

/test e2e-upgrade

dinhxuanvu · 2021-08-06T10:24:59Z

+	}
+
+	// make the update if possible
+	if _, err := client.OperatorsV1alpha1().CatalogSources(catsrc.GetNamespace()).Update(context.TODO(), catsrc, metav1.UpdateOptions{}); err != nil {


There is a bug in this function as this Update call only updates the spec, not the status. Another call UpdateStatus is needed here.
This is the downstream PR so this needs to be fixed upstream first.

kevinrizza · 2021-08-06T13:49:07Z

/test e2e-upgrade

kevinrizza · 2021-08-06T18:02:03Z

/test e2e-upgrade

kevinrizza · 2021-08-06T23:39:26Z

/test e2e-upgrade

kevinrizza · 2021-08-08T19:15:42Z

/test e2e-upgrade

kevinrizza · 2021-08-09T16:11:03Z

/bugzilla refresh

openshift-ci · 2021-08-09T16:11:05Z

@kevinrizza: An error was encountered searching for bug 1991662 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message.


could not unmarshal response body: invalid character '<' looking for beginning of value

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

Details

In response to this:

Bug 1991662: Catalog switcher (#2281)

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/test-infra repository.

openshift-ci · 2021-08-09T16:11:13Z

@kevinrizza: An error was encountered searching for bug 1991662 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message.


could not unmarshal response body: invalid character '<' looking for beginning of value

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

Details

In response to this:

/bugzilla 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 kubernetes/test-infra repository.

dinhxuanvu

/lgtm

openshift-ci · 2021-08-09T16:22:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dinhxuanvu, kevinrizza

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~~ [dinhxuanvu,kevinrizza]

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

davegord · 2021-08-09T17:14:17Z

/retest

davegord · 2021-08-09T18:58:44Z

/retest

openshift-ci · 2021-08-09T20:58:36Z

@kevinrizza: An error was encountered searching for external tracker bugs for bug 1991662 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message.


could not unmarshal response body: invalid character '<' looking for beginning of value

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

Details

In response to this:

Bug 1991662: Catalog switcher

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/test-infra repository.

OLM is unable to properly handle the case where a bundle author explicitly adds a hardcoded reference to a service account with a name that matches the name of the service account defined by the deployment spec in the CSV. This commit adds a validation method to ensure that the name of the service account in a bundle does not match deployment.Spec.Template.Spec.ServiceAccountName in the csv's spec Upstream-repository: api Upstream-commit: 0fe04f80e0102dc6973109a221a7babd6bee005e

openshift-ci Bot requested review from anik120 and dinhxuanvu July 30, 2021 17:57

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 30, 2021

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 30, 2021

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 30, 2021

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 31, 2021

Wrap catalog update test with retry logic (#2311)

82328e0

Upstream-repository: operator-lifecycle-manager Upstream-commit: d55593954671505ef56bf09eb2e7900069fedecb

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 2, 2021

dinhxuanvu reviewed Aug 6, 2021

View reviewed changes

kevinrizza changed the title ~~Catalog switcher (#2281)~~ Bug 1991662: Catalog switcher (#2281) Aug 9, 2021

kevinrizza changed the title ~~Bug 1991662: Catalog switcher (#2281)~~ Bug 1991662: Catalog switcher Aug 9, 2021

dinhxuanvu approved these changes Aug 9, 2021

View reviewed changes

openshift-ci Bot assigned dinhxuanvu Aug 9, 2021

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 9, 2021

openshift-ci Bot merged commit fb92eba into openshift:master Aug 9, 2021

Conversation

kevinrizza commented Jul 30, 2021

Uh oh!

timflannagan commented Jul 30, 2021

Uh oh!

timflannagan commented Jul 30, 2021

Uh oh!

timflannagan commented Jul 30, 2021

Uh oh!

timflannagan commented Jul 30, 2021

Uh oh!

davegord commented Jul 30, 2021

Uh oh!

timflannagan commented Jul 30, 2021

Uh oh!

davegord commented Jul 31, 2021

Uh oh!

davegord commented Jul 31, 2021

Uh oh!

kevinrizza commented Jul 31, 2021

Uh oh!

kevinrizza commented Aug 2, 2021

Uh oh!

kevinrizza commented Aug 4, 2021

Uh oh!

kevinrizza commented Aug 4, 2021

Uh oh!

kevinrizza commented Aug 4, 2021

Uh oh!

kevinrizza commented Aug 4, 2021

Uh oh!

davegord commented Aug 4, 2021

Uh oh!

kevinrizza commented Aug 5, 2021

Uh oh!

kevinrizza commented Aug 5, 2021

Uh oh!

kevinrizza commented Aug 5, 2021

Uh oh!

kevinrizza commented Aug 5, 2021

Uh oh!

kevinrizza commented Aug 5, 2021

Uh oh!

kevinrizza commented Aug 6, 2021

Uh oh!

dinhxuanvu Aug 6, 2021

Choose a reason for hiding this comment

Uh oh!

kevinrizza commented Aug 6, 2021

Uh oh!

kevinrizza commented Aug 6, 2021

Uh oh!

kevinrizza commented Aug 6, 2021

Uh oh!

kevinrizza commented Aug 8, 2021

Uh oh!

kevinrizza commented Aug 9, 2021

Uh oh!

openshift-ci Bot commented Aug 9, 2021

Uh oh!

openshift-ci Bot commented Aug 9, 2021

Uh oh!

dinhxuanvu left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Aug 9, 2021

Uh oh!

davegord commented Aug 9, 2021

Uh oh!

davegord commented Aug 9, 2021

Uh oh!

openshift-ci Bot commented Aug 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!