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

Fix bundle build workflow #153

Merged
merged 3 commits into from
Oct 24, 2023

Conversation

adam-cattermole
Copy link
Member

Porting of the changes from https://github.com/Kuadrant/authorino-operator/tree/v0.9.0 to fix the release workflow and manifests building.

Copy link
Collaborator

@guicassolato guicassolato left a comment

Choose a reason for hiding this comment

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

Let's also default to stable channel in the bundle annotations file?

operators.operatorframework.io.bundle.channels.v1: alpha

@adam-cattermole
Copy link
Member Author

Sure latest commit added where I ran make manifests bundle CHANNELS=stable DEFAULT_CHANNEL=stable

@adam-cattermole
Copy link
Member Author

adam-cattermole commented Oct 23, 2023

Actually, maybe I should set CHANNELS?=stable by default in the Makefile, the default with --overwrite behaviour from operator-sdk is to set it to alpha when not defined

@adam-cattermole
Copy link
Member Author

Oh maybe the same should be true for DEFAULT_CHANNEL - verify fails as it does not set either of these by default

@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2023

Codecov Report

Merging #153 (f5ff784) into main (aee73cf) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #153   +/-   ##
=======================================
  Coverage   62.85%   62.85%           
=======================================
  Files           1        1           
  Lines         735      735           
=======================================
  Hits          462      462           
  Misses        222      222           
  Partials       51       51           
Flag Coverage Δ
unit 62.85% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

guicassolato
guicassolato previously approved these changes Oct 23, 2023
Copy link
Collaborator

@guicassolato guicassolato left a comment

Choose a reason for hiding this comment

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

Approving this for now but hoping anytime soon we get more insights on the stable vs alpha default channel name debate.

Makefile Outdated
@@ -38,15 +38,13 @@ BUNDLE_IMG ?= $(IMAGE_TAG_BASE)-bundle:$(IMAGE_TAG)

# CHANNELS define the bundle channels used in the bundle.
# Add a new line here if you would like to change its default config. (E.g CHANNELS = "candidate,fast,stable")
ifneq ($(origin CHANNELS), undefined)
CHANNELS ?= stable
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've been banging my head with this change (discussed offline)...

Starting at v0.9.0, we define one OLM channel called stable. It used to be alpha in previous versions and, until this PR gets merged, it still is in the main branch, out of which we build the bundle and catalog :latest images.

On one hand, having the stable channel for the released versions as well as in main makes things easy and straightforward. It leaves alphacompletely in the past and ensures a more seamless UX between released and latest versions.

On the other hand, I wonder if semantically we might be sending the wrong message that latest == stable, and therefore if stable should be reserved to released versions of the operator only.

To exemplify, consider the following. Say you have:

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: operatorhubio-catalog
  namespace: authorino-operator
spec:
  sourceType: grpc
  image: quay.io/kuadrant/authorino-operator-catalog:v0.9.0
  displayName: Authorino Operator
---
apiVersion: operators.coreos.com/v1alpha1
kind: Subscription
metadata:
  name: authorino-operator
spec:
  channel: stable
  installPlanApproval: Automatic
  name: authorino-operator
  source: operatorhubio-catalog
  sourceNamespace: authorino-operator

And then, in the CatalogSource, you change to:

  image: quay.io/kuadrant/authorino-operator-catalog:latest

What would you expect to happen? With stable in the released version and alpha in main/latest, the “upgrade” from v0.9.0 to latest would fail. Whereas, with stable/stable, latest is rollout out without hiccups.

What’s the safest behaviour to enforce here? Would failing the upgrade from a released version to latest be a valid way to tell users that latest is not to be considered stable? Or, as a user, would you prefer to keep it simple with one single channel name and make the upgrade work out of the box?

@adam-cattermole
Copy link
Member Author

adam-cattermole commented Oct 24, 2023

I have changed the behaviour to default to alpha on the main branch. When we provide the DEFAULT_CHANNEL and CHANNELS as part of the release workflow the bundle.Dockerfile and bundle annotations will be updated to the correct channels.

If we want to build catalogs from main branch and test the upgrade from a previously released bundle version we need to specify stable for the channels above when building the bundle from main.

@adam-cattermole adam-cattermole merged commit 57023d1 into Kuadrant:main Oct 24, 2023
8 checks passed
@adam-cattermole adam-cattermole deleted the fix-release-workflow branch October 24, 2023 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants