Added Workflow job to update CFO image #398
Conversation
|
Other than those small adjustments, lgtm. Real nifty change to have it automatically update :) |
11efe5b to
944099c
Compare
|
Hey @Bobbins228 The tests are failing due to kustomize running from default, see ENV variable in the makefile. When running make deploy, this ensures the build from kustomize is from within the default directory. But with the updates in the pr, you're changes are within /config/manifests, which means for those to update the deployment the ENV would need to be set to manifests. To see this you can run ^^edit: commented at the same time as @astefanutti , see his :) |
|
/lgtm |
|
@sutaakar you OK with using |
|
It is ok for me. |
Right, I was actually surprised the deployment in the e2e tests pass, which made me realise it uses the |
|
@Bobbins228 @sutaakar Or we accept that |
|
I would prefer having manifest in |
3f10b02 to
98cf977
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
663dbb9 to
8626fd1
Compare
8626fd1 to
db8a160
Compare
3ebb376 to
7945b8b
Compare
85a691e to
3188af8
Compare
| @@ -34,7 +34,7 @@ spec: | |||
| containers: | |||
| - command: | |||
| - /manager | |||
| image: controller:latest | |||
| image: $(codeflare_operator_controller_image) | |||
There was a problem hiding this comment.
Shouldn't it be codeflare-operator-controller-image instead of codeflare_operator_controller_image?
There was a problem hiding this comment.
The codeflare_operator_controller_image is just a variable for the image in the params file.
We specify the replacement for $(codeflare_operator_controller_image) in the kustomization file
| @@ -198,7 +198,7 @@ uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified | |||
|
|
|||
| .PHONY: deploy | |||
| deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config. | |||
| cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG} | |||
| cd config/manager && IMAGE=$(IMG) perl -i -pe 's/codeflare-operator-controller-image=(.*)$$/codeflare-operator-controller-image=$$ENV{"IMAGE"}/' params.env | |||
There was a problem hiding this comment.
Is that change really needed?
There was a problem hiding this comment.
Yes we are using the command to update the params file so that the image variable codeflare-operator-controller-image is updated with the passed IMG
| @@ -0,0 +1 @@ | |||
| codeflare-operator-controller-image=quay.io/opendatahub/codeflare-operator:v1.0.1 | |||
There was a problem hiding this comment.
Do we have an agreement with the ODH operator team to change the name from odh-codeflare-operator-controller-image?
There was a problem hiding this comment.
Good catch I haven't informed anyone yet of this change. Will I just drop a message into the ODH DW channel?
There was a problem hiding this comment.
late for the party, but, if i understand this correctly, we are not going to keep CFO in the near future for ODH and RHOAI.
so the changes done in upstream(as project-codeflare) is irrelevant to ODH and RHOAI.
which means, purely from project-codeflare perspective, these changes are not needed?
There was a problem hiding this comment.
@zdtsw The CFO is still going to be part of ODH / RHOAI in the near future. It's possible we'll reconsider that in the future, but at the moment, it's preferable we operate as usual and consider that these changes are needed.
Issue link
Closes #392
What changes have been made
Added a job to the release workflow that will update the version of the CFO image to the latest version.
Commit job has been updated to commit
*.envfilesUpdated
config/manifests/kustomization.yamland added image stream filesVerification steps
Test the perl command for updating the image locally
Run
make deploy -e IMG=<YOUR_DEV_IMAGE> -e ENV="e2e"for KubernetesRun
make deploy -e IMG=<YOUR_DEV_IMAGE>for OpenShiftChecks