Skip to content

fix: Prefix image repository with image registry for provider images #1912

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

Jump to bottom
Merged
yiannistri merged 1 commit into from
Dec 10, 2025
Merged

fix: Prefix image repository with image registry for provider images #1912

yiannistri merged 1 commit into from
Dec 10, 2025
+113 −16

Conversation

@yiannistri
Copy link
Contributor

@yiannistri yiannistri commented Nov 24, 2025 •
edited
Loading

What this PR does / why we need it:
This PR adds a new feature flag (which is enabled by default) that controls whether Turtles will use the Rancher default registry when pulling provider images.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1904

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

@yiannistri yiannistri added kind/bug Something isn't working area/build-and-release Indicates issue or PR related to build or release labels Nov 24, 2025
@yiannistri yiannistri force-pushed the 1904-registry branch 2 times, most recently from d9e2019 to 2bfe40a Compare November 26, 2025 10:38
@kkaempf kkaempf moved this to PR to be reviewed in CAPI / Turtles Nov 26, 2025
@kkaempf kkaempf added this to the v2.13.1 milestone Nov 26, 2025
alexander-demicev
alexander-demicev reviewed Dec 5, 2025
View reviewed changes
@yiannistri yiannistri force-pushed the 1904-registry branch 12 times, most recently from 0615c2b to 52121f2 Compare December 8, 2025 13:52
@yiannistri yiannistri marked this pull request as ready for review December 8, 2025 15:12
@yiannistri yiannistri requested a review from a team as a code owner December 8, 2025 15:12
@yiannistri yiannistri force-pushed the 1904-registry branch 3 times, most recently from 9dee303 to 8518121 Compare December 8, 2025 16:06
anmazzotti
anmazzotti reviewed Dec 8, 2025
View reviewed changes
@yiannistri yiannistri force-pushed the 1904-registry branch 3 times, most recently from f053105 to 441c55c Compare December 9, 2025 07:42
@salasberryfin
Copy link
Contributor

salasberryfin commented Dec 9, 2025

Thanks @yiannistri.

We should probably think about the existing override configuration in config-prime.yaml. If I get this right, with this logic we're essentially applying equivalent overrides, as system-default-registry will point to Prime registry when running Rancher Prime, and Docker Hub for community (effectively system-default-registry is empty).

To make it more concise and avoid confusion, I'd consider removing the existing overrides from the clusterctl configuration file:

https://github.com/rancher/turtles/blob/a8d1450ea50afba50cc881b4426cc3dda42effd5/internal/controllers/clusterctl/config-prime.yaml#L55-#L72

Does this make sense?

@yiannistri
Copy link
Contributor Author

yiannistri commented Dec 9, 2025

@salasberryfin that makes sense to me. @anmazzotti I remember discussing this, wdyt about removing these overrides?

@anmazzotti
Copy link
Contributor

anmazzotti commented Dec 9, 2025 •
edited
Loading

@yiannistri @salasberryfin agreed the hardcoded image overrides are redundant at this point and can be removed in this PR.

@yiannistri yiannistri force-pushed the 1904-registry branch 3 times, most recently from f45a9ce to e076581 Compare December 9, 2025 12:32
salasberryfin
salasberryfin previously approved these changes Dec 9, 2025
View reviewed changes
Copy link
Contributor

@salasberryfin salasberryfin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @yiannistri. We should probably document (we can have a follow up) the default behavior, how overrides are applied, and why testing this is complicated when dealing with RC releases. Not sure where this information should go, though, as it probably isn't content for the user documentation.

@thehejik
Copy link
Collaborator

thehejik commented Dec 9, 2025 •
edited
Loading

Env:

  • rancher-prime 2.13.0-alpha8 which uses system-default-registry=stgregistry.suse.com
  • turtles image and chart built from this PR with TARGET_BUILD=prime
  • turtles installed within rancher provisioning by providing fake system chart repository
  • providers installed by using providers charts

The cluster-api image is correctly overriten to use current system-default-registry value so the capi-controller-manager was correctly trying to pull stgregistry.suse.com/rancher/cluster-api-controller:v1.10.6 but it failed on PullError as the image is not present there.

I also tried to provision usual providers and most of them defined in config-prime.yaml were correctly overwritten and available, but:

Please look bellow:

capv 		stgregistry.suse.com/rancher/cluster-api-vsphere-controller:v1.13.1 	present
capz 		stgregistry.suse.com/rancher/azureserviceoperator:v2.11.0 				missing, strange image name expected
capz		stgregistry.suse.com/rancher/cluster-api-azure-controller:v1.21.0		present
capa 		stgregistry.suse.com/rancher/cluster-api-aws-controller:v2.9.1 			present
capg 		stgregistry.suse.com/rancher/cluster-api-gcp-controller:v1.10.0 		present
core-api	stgregistry.suse.com/rancher/cluster-api-controller:v1.10.6 			missing
rke2-cp 	stgregistry.suse.com/rancher/cluster-api-provider-rke2-controlplane:v0.21.1 	present
rke2-bs 	stgregistry.suse.com/rancher/cluster-api-provider-rke2-bootstrap:v0.21.1 		present
caapf		ghcr.io/rancher/cluster-api-addon-provider-fleet:v0.12.0 					not overwritten???
EDIT: caapf fixed by latest commit e0e2992:
caapf		stgregistry.suse.com/rancher/cluster-api-addon-provider-fleet:v0.12.0		missing

Other images not defined in prime-config:
capd 		gcr.io/k8s-staging-cluster-api/capd-manager:v1.10.6
kubeadm 	registry.rancher.com/rancher/kubeadm-bootstrap-controller:v1.10.6
kubeadm 	registry.rancher.com/rancher/kubeadm-control-plane-controller:v1.10.6

@anmazzotti
Copy link
Contributor

anmazzotti commented Dec 9, 2025

@thehejik regarding capz, there will be 2 pods running in the namespace, one is the azureserviceoperator, but there should also be the cluster-api-azure-controller pod running

@thehejik
Copy link
Collaborator

thehejik commented Dec 9, 2025

@thehejik regarding capz, there will be 2 pods running in the namespace, one is the azureserviceoperator, but there should also be the cluster-api-azure-controller pod running

Right, I overlooked the second one, table above updated - it is present on stgregistry, thanks

@thehejik thehejik mentioned this pull request Dec 9, 2025
Open
@thehejik
Copy link
Collaborator

thehejik commented Dec 10, 2025 •
edited
Loading

caapf image override has been fixed in commit e0e2992 but the image is missing (report above updated):

caapf		stgregistry.suse.com/rancher/cluster-api-addon-provider-fleet:v0.12.0		missing

@yiannistri yiannistri changed the title fix: Prefix image repository with image registry for core CAPI fix: Prefix image repository with image registry for provider images Dec 10, 2025
thehejik
thehejik approved these changes Dec 10, 2025
View reviewed changes
Copy link
Collaborator

@thehejik thehejik left a comment

Choose a reason for hiding this comment

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

From my point of view it's working as expected now. Thanks!

alexander-demicev
alexander-demicev reviewed Dec 10, 2025
View reviewed changes
Copy link
Member

@alexander-demicev alexander-demicev left a comment

Choose a reason for hiding this comment

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

thanks! Just one more minor comment

@yiannistri yiannistri merged commit 53acca9 into rancher:main Dec 10, 2025
15 checks passed
@github-project-automation github-project-automation bot moved this from PR to be reviewed to Done in CAPI / Turtles Dec 10, 2025
@yiannistri yiannistri deleted the 1904-registry branch December 10, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexander-demicev alexander-demicev alexander-demicev left review comments

@salasberryfin salasberryfin salasberryfin approved these changes

@thehejik thehejik thehejik approved these changes

@anmazzotti anmazzotti anmazzotti approved these changes

Assignees

@yiannistri yiannistri

Labels

area/airgapped area/build-and-release Indicates issue or PR related to build or release kind/bug Something isn't working

Projects

CAPI / Turtles
Status: Done

Milestone

v2.13.1

Development

Successfully merging this pull request may close these issues.

Leverage systemDefaultRegistry setting when creating clusterctl config

6 participants

@yiannistri @salasberryfin @anmazzotti @thehejik @alexander-demicev @kkaempf