Skip to content

add new github action which tags and builds a new operator #110

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

Merged

Conversation

KPostOffice
Copy link
Collaborator

fixes #98

@openshift-ci
Copy link

openshift-ci bot commented May 24, 2023

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

Makefile Outdated
Comment on lines 9 to 10
PREVIOUS_VERSION ?= 0.0.1
VERSION ?= 0.0.2
Copy link
Member

Choose a reason for hiding this comment

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

Ran make bundle -e PREVIOUS_VERSION=0.0.3 -e VERSION=0.0.4. No error.
To make the next release easier, should we preset the envs to:

Suggested change
PREVIOUS_VERSION ?= 0.0.1
VERSION ?= 0.0.2
PREVIOUS_VERSION ?= 0.0.3
VERSION ?= 0.0.4

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you think about making them explicitly dev? Something like:

VERSION ?= 0.0.0-dev
PREVIOUS_VERSION ?= 0.0.0-dev  # maybe even creating a conditional instead in the case that it is unset

Copy link
Member

@tedhtchang tedhtchang May 30, 2023

Choose a reason for hiding this comment

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

I think VERSION ?= 0.0.0-dev should be ok. PREVIOUS_VERSION may safer to be an existing bundle release judging by some examples here which implies the replaces is an version from an update graph which maybe built from existing CSVs.

Comment on lines 40 to 46
- name: Install operator-sdk
run: |
export ARCH=$(case $(uname -m) in x86_64) echo -n amd64 ;; aarch64) echo -n arm64 ;; *) echo -n $(uname -m) ;; esac)
export OS=$(uname | awk '{print tolower($0)}')
export OPERATOR_SDK_DL_URL=https://github.com/operator-framework/operator-sdk/releases/download/v1.27.0
curl -LO ${OPERATOR_SDK_DL_URL}/operator-sdk_${OS}_${ARCH}
chmod +x operator-sdk_${OS}_${ARCH} && sudo mv operator-sdk_${OS}_${ARCH} /usr/local/bin/operator-sdk
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason not using make install-operator-sdk ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was using an existing git action. https://github.com/project-codeflare/codeflare-operator/blob/main/.github/workflows/operator-image.yml I can see if I can do this the way that you suggested

Copy link
Member

@tedhtchang tedhtchang left a comment

Choose a reason for hiding this comment

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

Hi @KPostOffice I don't know if I could test out the the git action because I don't have the credentials to the the Rh registry. Otherwise looks good just some nit picks.

@KPostOffice
Copy link
Collaborator Author

Hi @KPostOffice I don't know if I could test out the the git action because I don't have the credentials to the the Rh registry. Otherwise looks good just some nit picks.

Haven't tested yet. I will try the changes on my fork.

@KPostOffice KPostOffice force-pushed the issue98-v1 branch 2 times, most recently from 4899526 to 771b5a6 Compare May 31, 2023 15:13
@tedhtchang
Copy link
Member

tedhtchang commented May 31, 2023

@KPostOffice I could not test the the new github workflow. The make bundle /lgtm

@anishasthana
Copy link
Contributor

What do folks think of switching over https://github.com/project-codeflare/codeflare-operator/blob/main/Dockerfile#L2 to be one of the registry.access.redhat.com/ubi8/go-toolset:1.18.9-8? That oughta get around the need to authenticate to the red hat registry.

this makes the images easier to build for those outside of the red hat organization

Signed-off-by: Kevin <[email protected]>
@KPostOffice
Copy link
Collaborator Author

registry.access.redhat.com/ubi8/go-toolset:1.18.9-8

More than happy to do this. I can then also remove the auth step from the GitHub actions

Copy link
Collaborator

@jbusche jbusche left a comment

Choose a reason for hiding this comment

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

LGTM - Tested together with Kevin on my fork, the github actions worked beautifully and pushed the candidate release to my quay repo.

@openshift-ci
Copy link

openshift-ci bot commented Jun 5, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jbusche

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:

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

@openshift-ci openshift-ci bot added the approved label Jun 5, 2023
@jbusche
Copy link
Collaborator

jbusche commented Jun 5, 2023

/lgtm

@openshift-merge-robot openshift-merge-robot merged commit be42693 into project-codeflare:main Jun 5, 2023
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.

Add replaces to the base CSV template and patch the value with the previous version
6 participants