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

olm(channel): change from rhtas-1-0 to stable-1-0 #359

Merged
merged 2 commits into from
May 8, 2024

Conversation

lance
Copy link
Member

@lance lance commented May 6, 2024

This commit modifies the channels we advertise for the operator, removing the rhtas- prefix in favor of a stable- prefix, indicating the major/minor version for that particular stable line. This channel naming is based on the olm documentation best practices found at: https://olm.operatorframework.io/docs/best-practices/channel-naming/#example-3-recommended-option-is-the-most-common-scenarios

I did not include a fast-1-0 or candidate-1-0, however the current configuration does not prevent these channels from being used in the future. At this point, I don't think we have a strong enough cadence to be using fast and candidate, however we may want to consider that for the July release, or at least the EOY release.

If we want to follow the recommended semver naming conventions, then we should consider the July release to be v1.1.0, given that it should introduce new functionality with SECURESIGN-720 and possibly others Jiras as well (key rotation, independently deployable services, etc).

If this PR is merged, the user would see only two channels when installing: stable and stable-1-0. They would (for now) contain the exact same versions. But ultimately, as newer minor versions are released, they will all appear in stable, but stable-1-0 will only contain 1.0.0, 1.0.1, and any other 1.0.* releases we produce.

Do not merge this commit without discussion. I think we really want to get it right and not have to make further changes.

Copy link

openshift-ci bot commented May 6, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the approved label May 6, 2024
@lance lance requested a review from osmman May 6, 2024 18:52
Copy link
Contributor

@Gregory-Pereira Gregory-Pereira left a comment

Choose a reason for hiding this comment

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

/lgtm just 1 question

@@ -193,7 +193,7 @@ metadata:
features.operators.openshift.io/token-auth-aws: "false"
features.operators.openshift.io/token-auth-azure: "false"
features.operators.openshift.io/token-auth-gcp: "false"
operators.operatorframework.io/builder: operator-sdk-v1.32.0
operators.operatorframework.io/builder: operator-sdk-v1.34.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are bumping this dont we also need to bump the operator sdk verison in the makefile

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably - I just ran make bundle 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated the makefile and also noticed an old alpha annotation hanging around that I changed to stable

Copy link
Contributor

@Gregory-Pereira Gregory-Pereira left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link
Contributor

@JasonPowr JasonPowr left a comment

Choose a reason for hiding this comment

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

/lgtm

The CI is currently failing because this is opened of a fork, and not a direct branch, just FYI

https://github.com/securesign/secure-sign-operator/actions/runs/8974908261/job/24648503582?pr=359#step:4:1

Copy link
Contributor

@osmman osmman left a comment

Choose a reason for hiding this comment

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

I agree with new format of channel naming.

I found few places with old version v1.32.0 of operator sdk and alpha channel. Can you please to update them too.

@openshift-ci openshift-ci bot removed the lgtm label May 7, 2024
@lance lance marked this pull request as ready for review May 7, 2024 11:24
@openshift-ci openshift-ci bot requested review from cooktheryan and lkatalin May 7, 2024 11:24
@@ -73,7 +73,7 @@ LABEL operators.operatorframework.io.bundle.manifests.v1=manifests/
LABEL operators.operatorframework.io.bundle.metadata.v1=metadata/
LABEL operators.operatorframework.io.bundle.package.v1=rhtas-operator
LABEL operators.operatorframework.io.bundle.channels.v1=alpha
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LABEL operators.operatorframework.io.bundle.channels.v1=alpha
LABEL operators.operatorframework.io.bundle.channels.v1=stable

@lance
Copy link
Member Author

lance commented May 8, 2024

/lgtm

The CI is currently failing because this is opened of a fork, and not a direct branch, just FYI

https://github.com/securesign/secure-sign-operator/actions/runs/8974908261/job/24648503582?pr=359#step:4:1

Always forget about that...

This commit modifies the channels we advertise for the operator,
removing the `rhtas-` prefix in favor of a `stable-` prefix, indicating
the major/minor version for that particular stable line. This channel
naming is based on the olm documentation best practices found at:
https://olm.operatorframework.io/docs/best-practices/channel-naming/#example-3-recommended-option-is-the-most-common-scenarios

I did not include a `fast-1-0` or `candidate-1-0`, however the current
configuration does not prevent these channels from being used in the
future. At this point, I don't think we have a strong enough cadence to
be using `fast` and `candidate`, however we may want to consider that
for the July release, or at least the EOY release.

If we want to follow the recommended semver naming conventions, then we
should consider the July release to be v1.1.0, given that it _should_
introduce new functionality with SECURESIGN-720 and possibly others
Jiras as well (key rotation, independently deployable services, etc).

If this PR is merged, the user would see only two channels when
installing `stable` and `stable-1-0`. They would (for now) contain the
exact same versions. But ultimately, as newer minor versions are
released, they will _all_ appear in `stable`, but `stable-1-0` will only
contain 1.0.0, 1.0.1, and any other 1.0.* releases we produce.

Do not merge this commit without discussion. I think we really want to get
it right and not have to make further changes.

Signed-off-by: Lance Ball <[email protected]>
@lance lance force-pushed the channel-conventions branch from e3a4e43 to 5725be7 Compare May 8, 2024 13:11
Copy link
Contributor

@JasonPowr JasonPowr left a comment

Choose a reason for hiding this comment

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

Seems fine, just too things I noticed

fbc/generate-fbc.sh Outdated Show resolved Hide resolved
fbc/generate-fbc.sh Outdated Show resolved Hide resolved
Signed-off-by: Lance Ball <[email protected]>
Copy link
Contributor

@JasonPowr JasonPowr left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm label May 8, 2024
Copy link

openshift-ci bot commented May 8, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Gregory-Pereira, JasonPowr, lance

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 [Gregory-Pereira,JasonPowr,lance]

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

@lance lance merged commit 0b2480d into securesign:main May 8, 2024
10 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants