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

Use eck-operator UBI image when ubiOnly=true. #7486

Merged
merged 7 commits into from
Jan 23, 2024

Conversation

naemono
Copy link
Contributor

@naemono naemono commented Jan 18, 2024

resolves #7485

Ensure we use the -ubi operator image when ubiOnly is set to true.

❯ helm template eck-operator . -s templates/statefulset.yaml | yq '.spec.template.spec.containers[0].image'
docker.elastic.co/eck/eck-operator:2.12.0-SNAPSHOT

❯ helm template eck-operator . -s templates/statefulset.yaml --set=config.ubiOnly=true | yq '.spec.template.spec.containers[0].image'
docker.elastic.co/eck/eck-operator-ubi:2.12.0-SNAPSHOT

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono added the >bug Something isn't working label Jan 18, 2024
@thbkrkr
Copy link
Contributor

thbkrkr commented Jan 18, 2024

We don't build -ubi snapshot of the operator image today (docker.elastic.co/eck/eck-operator-ubi:2.12.0-SNAPSHOT). We need to identify the type of trigger that requires the image and update BUILD_FLAVORS in the operator-image.sh script based on that.

Example if it is only for nightly main builds:

diff --git a/.buildkite/scripts/common/operator-image.sh b/.buildkite/scripts/common/operator-image.sh
index 64d62b073..4c4e81ba3 100644
--- a/.buildkite/scripts/common/operator-image.sh
+++ b/.buildkite/scripts/common/operator-image.sh
@@ -50,7 +50,8 @@ operator::set_build_flavors_var() {
     if [[ "${BUILD_FLAVORS:-}" == "" ]]; then
         case $trigger in
             tag-*)           BUILD_FLAVORS="eck,eck-dev,eck-fips,eck-ubi,eck-ubi-fips" ;;
-            *-main)          BUILD_FLAVORS="eck,eck-dev" ;;
+            merge-main)      BUILD_FLAVORS="eck,eck-dev" ;;
+            nightly-main)    BUILD_FLAVORS="eck,eck-dev,eck-ubi" ;;
             *-test-snapshot) BUILD_FLAVORS="eck,eck-dev" ;;
             pr-*|merge-xyz)  BUILD_FLAVORS="eck" ;;
             dev)             BUILD_FLAVORS="dev" ;;

@naemono
Copy link
Contributor Author

naemono commented Jan 18, 2024

We don't build -ubi snapshot of the operator image today (docker.elastic.co/eck/eck-operator-ubi:2.12.0-SNAPSHOT). We need to identify the type of trigger that requires the image and update BUILD_FLAVORS in the operator-image.sh script based on that.

Thank you. Upon looking through these options, I suspect nighty-main, tag-final, and tag-bc probably make the most sense, as we want to do this testing during BC iterations, after a release, and are discussing adding something in the nightly. I'll get these updated.

@naemono
Copy link
Contributor Author

naemono commented Jan 18, 2024

We don't build -ubi snapshot of the operator image today

This just happened to be me testing on a branch where snapshot was set, and no way was intended to mean that we should build these on for all SNAPSHOT builds...

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono requested a review from thbkrkr January 18, 2024 19:37
Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

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

Code LGTM but are we okay with changing the existing behavior by forcing the use of the -ubi image when the Helm config.ubiOnly value is enabled?

Copy link
Contributor

@barkbay barkbay 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 maybe update the comment here if this setting does not only control the stack images:

# ubiOnly specifies whether the operator will use only UBI container images to deploy Elastic Stack applications. UBI images are only available from 7.10.0 onward.

Let's also create an issue so we don't forget to mention this change in the next release notes?

Other than that, I think I'm ok with this change, but @thbkrkr's comment is relevant, curious to know if there are other opinions.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono
Copy link
Contributor Author

naemono commented Jan 23, 2024

Let's also create an issue so we don't forget to mention this change in the next release notes?

#7485 exists and is tagged with the next release. Is this what you were intending?

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@barkbay
Copy link
Contributor

barkbay commented Jan 23, 2024

#7485 exists and is tagged with the next release. Is this what you were intending?

Sorry, I meant an issue to maybe add a short sentence to mention this change in the highlights.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono
Copy link
Contributor Author

naemono commented Jan 23, 2024

Sorry, I meant an issue to maybe add a short sentence to mention this change in the highlights.

I've added the release-highlight tag to that issue as well.

@naemono naemono requested a review from thbkrkr January 23, 2024 14:18
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@barkbay
Copy link
Contributor

barkbay commented Jan 23, 2024

I've added the release-highlight tag to that issue as well.

I have to confess that as a release manager I'm not sure I've ever used it 😬 . We should maybe update our release process.

@naemono
Copy link
Contributor Author

naemono commented Jan 23, 2024

I've added the release-highlight tag to that issue as well.

I have to confess that as a release manager I'm not sure I've ever used it 😬 . We should maybe update our release process.

I just updated the documentation/template mentioning this tag during writing of the release highlights.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono enabled auto-merge (squash) January 23, 2024 16:43
@naemono naemono merged commit bfae9f6 into elastic:main Jan 23, 2024
5 checks passed
@naemono naemono deleted the eck-operator-helm-chart-use-ubi-image branch January 23, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working v2.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ECK Operator helm chart should use UBI image for operator when ubiOnly is true
3 participants