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.

Sign up for GitHub

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

ROX-22474: push Konflux images to quay.io/rhacs-eng #1674

Merged
merged 2 commits into from
May 21, 2024
Merged

ROX-22474: push Konflux images to quay.io/rhacs-eng #1674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8389cd2
ROX-22474: push Konflux images to quay.io/rhacs-eng
tommartensen May 15, 2024
2c31675
push both images to collector, but with -slim, -latest suffix
tommartensen May 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .tekton/collector-pull-request.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ spec:
- name: image-expires-after
value: '13w'
- name: output-image-repo
value: quay.io/redhat-user-workloads/rh-acs-tenant/acs/collector
value: quay.io/rhacs-eng/collector
- name: path-context
value: .
- name: revision
Expand Down Expand Up @@ -292,7 +292,7 @@ spec:
- name: build-container
params:
- name: IMAGE
value: $(params.output-image-repo):$(tasks.determine-image-tag.results.image-tag)
value: $(params.output-image-repo):$(tasks.determine-image-tag.results.image-tag)-latest
- name: DOCKERFILE
value: $(params.dockerfile)
- name: CONTEXT
Expand Down
4 changes: 2 additions & 2 deletions .tekton/collector-push.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ spec:
# TODO(ROX-20230): make release images not expire.
value: '13w'
- name: output-image-repo
value: quay.io/redhat-user-workloads/rh-acs-tenant/acs/collector
value: quay.io/rhacs-eng/collector
- name: path-context
value: .
- name: revision
Expand Down Expand Up @@ -292,7 +292,7 @@ spec:
- name: build-container
params:
- name: IMAGE
value: $(params.output-image-repo):$(tasks.determine-image-tag.results.image-tag)
value: $(params.output-image-repo):$(tasks.determine-image-tag.results.image-tag)-latest
- name: DOCKERFILE
value: $(params.dockerfile)
- name: CONTEXT
Expand Down
4 changes: 2 additions & 2 deletions .tekton/collector-slim-pull-request.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ spec:
- name: image-expires-after
value: '13w'
- name: output-image-repo
value: quay.io/redhat-user-workloads/rh-acs-tenant/acs/collector-slim
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please remind me, will main, roxctl and operator images think it is a release build in the sense of this code block here https://github.com/stackrox/stackrox/blob/896452897dbc54bb60183782731dda95766e3a20/pkg/images/defaults/flavor.go#L103-L112?
If yes, the repo is fine. If no, -slim should be part of the tag, not repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All three images have GOTAGS=release set, like https://github.com/stackrox/stackrox/blob/master/image/rhel/konflux.Dockerfile#L85.

If I understand this correctly, https://github.com/stackrox/stackrox/blob/896452897dbc54bb60183782731dda95766e3a20/pkg/images/defaults/flavor.go#L107 is true.

version.GetAllVersionsUnified would set the collector & scanner versions to the same as main (make tag of stackrox/stackrox instead of {COLLECTOR, SCANNER}_VERSION (https://github.com/stackrox/stackrox/blob/master/pkg/version/version.go#L72-L77)), BUT this is overriden in L109, L111, L128.

https://github.com/stackrox/stackrox/blob/master/pkg/version/version.go#L72-L77

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TLDR: I think it's fine and the assumption that SCANNER_VERSION and COLLECTOR_VERSION tags will be generated should also hold.

Copy link
Contributor

Choose a reason for hiding this comment

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

BUT this is overriden in L109, L111, L128

Overridden to what? GetAllVersionsUnified will return MainVersion there.

With GOTAGS=release:

  • quay.io/rhacs-eng/collector:<MainVersion>
  • quay.io/rhacs-eng/collector-slim:<MainVersion>

Without:

  • quay.io/rhacs-eng/collector:<CollectorVersion>-latest
  • quay.io/rhacs-eng/collector:<CollectorVersion>-slim

"<CollectorVersion>" is what we get by running make tag in stackrox/collector repo.
"<MainVersion>" is make tag result in stackrox/stackrox, i.e. a retagged image. https://github.com/stackrox/stackrox/blob/7690ea6ed07fe68b85aac8810b32c9d7b640e8fb/.github/workflows/build.yaml#L400-L408

We don't have a way to retag in Konflux unless we write a custom task which I at the moment doubt is the right choice.

-slim and -latest in suffix were introduced as a compatibility thing to not break old workflows. Maybe this is no longer necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think, the simplest for MVP^2 may be to switch the release off, push to -latest+-slim, and plan sorting out release in GA scope.

Copy link
Contributor Author

@tommartensen tommartensen May 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think, the simplest for MVP^2 may be to switch the release off, push to -latest+-slim, and plan sorting out release in GA scope.

IMO we can merge stackrox/stackrox#10824, (this can already be done without merging)

We can deploy a Central cluster and see what it would generate for collector(-slim). It might just work.

Copy link
Contributor

@msugakov msugakov May 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I realized yesterday that we might have to -fast in that file

We should not add suffixes to COLLECTOR_VERSION or SCANNER_VERSION because it can break builds outside of Konflux and/or product built outside of Konflux. We should look for other solution (overriding BUILD_TAG was my first candidate) - https://issues.redhat.com/browse/ROX-19958

So there are actually no images like quay.io/rhacs-eng/collector-slim:<CollectorVersion>.

Correct! And I think it proves my point, doesn't it?

Is the development_build flavor && releaseBuild path unused?

I believe it executes for nightly builds and any other tagged builds.

We can deploy a Central cluster and see what it would generate for collector(-slim).

There's a quicker way to check:

$ cd tmp
$ podman run --rm -it --entrypoint=/bin/bash -u 0:0 -v ./:/tmp quay.io/redhat-user-workloads/rh-acs-tenant/acs/roxctl:4.4.x-685-g7690ea6ed0-fast
$ roxctl helm output secured-cluster-services --output-dir /tmp/chart-konflux --remove --image-defaults=development_build
INFO:	Written Helm chart secured-cluster-services to directory "/tmp/chart-konflux"
$ exit
$ /assets/downloads/cli/roxctl-linux helm output secured-cluster-services --output-dir /tmp/chart-gha --remove --image-defaults=development_build
INFO:	Written Helm chart secured-cluster-services to directory "/tmp/chart-gha"
$ exit

(I used some successful build on master)

Then, study internal/defaults/50-images.yaml in both dirs.

/tmp/chart-konflux/internal/defaults/50-images.yaml has:

image:
  registry: quay.io/rhacs-eng
  main:
    name: main
    pullPolicy: IfNotPresent
  collector:
    name: collector
    slimName: collector-slim
# ...
image:
  collector:
    {{- if ._rox.collector.slimMode }}
    tag: "4.4.x-685-g7690ea6ed0"
    pullPolicy: IfNotPresent
    {{- else }}
    tag: "4.4.x-685-g7690ea6ed0"
    pullPolicy: Always
    {{- end }}

/tmp/chart-gha/internal/defaults/50-images.yaml:

image:
  registry: quay.io/rhacs-eng
  main:
    name: main
    pullPolicy: IfNotPresent
  collector:
    name: collector
    slimName: collector
# ...
image:
  collector:
    {{- if ._rox.collector.slimMode }}
    tag: "3.18.x-221-g580b954a9a-slim"
    pullPolicy: IfNotPresent
    {{- else }}
    tag: "3.18.x-221-g580b954a9a-latest"
    pullPolicy: Always
    {{- end }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you notice that scanner.tag is also the <MainVersion>, not <ScannerVersion>?

  scanner:
    name: scanner-slim
    tag: 4.4.x-685-g7690ea6ed0

I have to think about the other points..

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, that's the "unified versions" case of release builds, I expected it (we came up with this code in the Maple team while working on that image flavor stuff that occurred after onboarding all containers to CPaaS).

Copy link
Contributor Author

@tommartensen tommartensen May 16, 2024 •
edited by msugakov
Loading

Choose a reason for hiding this comment

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

From discussion in Weekly:

value: quay.io/rhacs-eng/collector
- name: path-context
value: .
- name: revision
Expand Down Expand Up @@ -288,7 +288,7 @@ spec:
- name: build-container
params:
- name: IMAGE
value: $(params.output-image-repo):$(tasks.determine-image-tag.results.image-tag)
value: $(params.output-image-repo):$(tasks.determine-image-tag.results.image-tag)-slim
- name: DOCKERFILE
value: $(params.dockerfile)
- name: CONTEXT
Expand Down
4 changes: 2 additions & 2 deletions .tekton/collector-slim-push.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ spec:
# TODO(ROX-20230): make release images not expire.
value: '13w'
- name: output-image-repo
value: quay.io/redhat-user-workloads/rh-acs-tenant/acs/collector-slim
value: quay.io/rhacs-eng/collector
- name: path-context
value: .
- name: revision
Expand Down Expand Up @@ -288,7 +288,7 @@ spec:
- name: build-container
params:
- name: IMAGE
value: $(params.output-image-repo):$(tasks.determine-image-tag.results.image-tag)
value: $(params.output-image-repo):$(tasks.determine-image-tag.results.image-tag)-slim
- name: DOCKERFILE
value: $(params.dockerfile)
- name: CONTEXT
Expand Down
Loading