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

ROX-18384: remove slim images from upstream #1963

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Stringy
Copy link
Collaborator

@Stringy Stringy commented Nov 18, 2024

Description

Since we have deprecated full images from Collector, we now want to drop the -slim tag for all images, since all image flavours are now inherently slim.

This PR removes upstream slim image tagging from Collector.

There are sibling PRs, which I will list here as they become available, that remove slim image use in other areas (downstream builds and the main repo)

Checklist

  • Investigated and inspected CI test results
  • Updated documentation accordingly

Automated testing

  • Added unit tests
  • Added integration tests
  • Added regression tests

If any of these don't apply, please comment below.

Testing Performed

CI should be enough.

@Stringy Stringy requested a review from a team as a code owner November 18, 2024 09:55
Copy link
Collaborator

@Molter73 Molter73 left a comment

Choose a reason for hiding this comment

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

Small nit, we probably want to rename this job by dropping slim from it
image

Other than that, LGTM! Godspeed full images, you will not be missed.

@@ -123,11 +109,9 @@
msg:
- "Pushed the following images:"
- " {{ collector_image }}-{{ arch }}"
- " {{ collector_image }}-{{ arch }}-slim"
- " {{ collector_image }}-{{ arch }}-base"
- " {{ collector_image }}-{{ arch }}-latest"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocking comment and not aiming to be delay the PR by becoming a reviewer.

I recalled this our conversation https://redhat-internal.slack.com/archives/CFMQ5C2TT/p1733309587336659

Your changes in https://github.com/stackrox/stackrox/pull/13350/files#diff-9f1c5124b529a407693ba184291c9797252978fe4ce8770e1ddec20d811efd69L104 allow getting rid of publishing -latest.

Also, I believe -base wasn't used by StackRox. IIRC, it was created by the Collector build pipelines as something intermediate before drivers are injected and it becomes "the full collector".

@@ -25,7 +25,7 @@ $ make image

Copy link
Contributor

Choose a reason for hiding this comment

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

(not about this line)
I suppose what some of my comments allude to is the need for more testing in different version combinations.
Such as:

  • New Central old roxctl, can roxctl generate sensor manifests and can that be successfully deployed? In both versions of slim=true/false.
  • Old Central, new roxctl, can sensor manifests be generated and successfully deployed?
  • Get old manifest-based secured cluster connected to new Central (probably start both old and then upgrade only Central). Does the UI display all things as expected? Can the secured cluster download probes through Central?
  • Get old Helm-based and Operator-based secured clusters connected to new Central. UI fine? Functioning ok?
  • Get old Central and connect new Helm-based and Operator-based secured clusters. All works well, including UI?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants