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

Run UBI-minimal FVs in Typha #9710

Merged

Conversation

aaaaaaaalex
Copy link
Contributor

Description

Related issues/PRs

Todos

  • Tests
  • Documentation
  • Release note

Release Note

TBD

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

  • docs-pr-required: This change requires a change to the documentation that has not been completed yet.
  • docs-completed: This change has all necessary documentation completed.
  • docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

  • release-note-required: This PR has user-facing changes. Most PRs should have this label.
  • release-note-not-required: This PR has no user-facing changes.

Other optional labels:

  • cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
  • needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

@aaaaaaaalex aaaaaaaalex added docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact labels Jan 14, 2025
@aaaaaaaalex aaaaaaaalex requested a review from a team as a code owner January 14, 2025 14:23
@marvin-tigera marvin-tigera added this to the Calico v3.30.0 milestone Jan 14, 2025
@aaaaaaaalex aaaaaaaalex requested a review from fasaxc January 14, 2025 14:23
@aaaaaaaalex aaaaaaaalex changed the title Run UBI-minimal FVs Run UBI-minimal FVs in Typha Jan 14, 2025
- ../.semaphore/run-and-monitor make-fv-ubi.log make clean image fv
env_vars:
- name: CALICO_BASE
value: registry.access.redhat.com/ubi8/ubi-minimal
Copy link
Member

Choose a reason for hiding this comment

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

Could you make this is a special "use ubi" value and do the switch to the exact image in the makefile? Just thinking that we already have registry.access.redhat.com/ubi8/ubi-minimal in metadata.mk

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@aaaaaaaalex aaaaaaaalex requested a review from fasaxc January 14, 2025 15:34
@@ -262,7 +262,11 @@ REPO_ROOT := $(shell git rev-parse --show-toplevel)
CERTS_PATH := $(REPO_ROOT)/hack/test/certs

# The image to use for building calico/base-dependent modules (e.g. apiserver, typha).
ifdef USE_UBI_AS_CALICO_BASE
CALICO_BASE ?= $(UBI_IMAGE)
Copy link
Member

Choose a reason for hiding this comment

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

Will this be the -minimal version that you were using before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we seem to use ubi-minimal in all these areas

@aaaaaaaalex aaaaaaaalex force-pushed the test-ubi-minimal-sem-run branch from b1bbb26 to ffbcc86 Compare January 14, 2025 15:40
@aaaaaaaalex aaaaaaaalex force-pushed the test-ubi-minimal-sem-run branch from ffbcc86 to b3d13bb Compare January 14, 2025 16:06
- name: "Typha: UT and FV tests on UBI-minimal"
commands:
- ../.semaphore/run-and-monitor make-fv-ubi.log make clean image fv
env_vars:
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 you can set the CALICO_BASE variable directly in the new pipeline rather than introducing a new USE_UBI_AS_CALICO_BASE flag. You can add additional checks before typha UT/FVs to make sure that the base is indeed switched.

env_vars:
  - name: CALICO_BASE
    value: registry.access.redhat.com/ubi8/ubi-minimal:latest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I had, but I re-did it to satisfy: #9710 (comment)

@aaaaaaaalex aaaaaaaalex merged commit e24eda3 into projectcalico:master Jan 15, 2025
4 checks passed
tomastigera pushed a commit to ioworker0/calico that referenced this pull request Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants