dashboard icon indicating copy to clipboard operation
dashboard copied to clipboard

We don't use cluster level registry for deploying Rancher helm charts

Open gaktive opened this issue 3 years ago • 9 comments

The majority of Rancher charts support the helm value global.cattle.systemDefaultRegistry (also note that our code also allows for global.systemDefaultRegistry` - this can be used to change the registry that images are pulled from when installing a chart. This allows us to support air-gapped installed.

There is a global setting setting-default-registry that the user can change through the UI.

Today, we will pass the value specified by this global setting to the global.cattle.systemDefaultRegistry helm chart value, if not already set by the user (e.g. by editing the values as yaml) - see shell/pages/c/_cluster/apps/charts/install.vue.

When a user provisions a new cluster, they can specify a custom registry - this is used to pull the images when provisioning, but is also stored on the cluster object - this is referred to as the cluster level registry.

The change required here is to allow the user to choose which registry to use when installing a Rancher chart.

As I see it:

  • For the Helm chart install, if the chart has either the global.cattle.systemDefaultRegistry or global.systemDefaultRegistry values (note the default is empty, so we need to check for presence of the keys), then we should provide new UI in the install experience to allow the user to specify the registry.
  • I suggest adding a new block of UI to the first step of the install process (Metadata) - note, it could fit in the next step as a new tab, but not all charts have values, so some just display a yaml editor for second step, so the only common place is the first step.
  • Edited plan: After speaking with Kenneth on Oct 4, the plan is no longer to add radio buttons allowing the user to select a registry because most users installing apps should not need to worry about the default registries, as that is more in the domain of admins. The new plan is to have a single checkbox that allows the user to input a custom registry, with a tooltip that explains what the default is if they do not select a custom one.

The cluster-level registry is available on the cluster object in spec.rkeConfig.machineSelectorConfig.system-default-registry.

Old notes below:

As the registry passed to feature helm charts is done in the UI, we need to expose what is the registry that will populate the "system-default-registry" for the helm chart deployments and allow the ability to update/change it from the UI. Presently, we default to grabbing charts from the default Rancher repo but we would need the ability to toggle to another repo.

This would require coordination with the backend (Team 2).

gaktive avatar Sep 16 '22 15:09 gaktive

(3) Use Custom Container Registry for Rancher system images - with an input box to allow the user to enter a URL

Since number 1 does not support credentials, but number 2 DOES support credentials, I have some questions about number 3 -

  • Can you confirm that credentials won't be supported for number 3?
  • Cluster explorer already has a form for creating private registries (or to be more pedantic, a secret of type registry). Do we want to reuse the existing private registry creation form?
  • And do we want the user to be able to select an existing secret of type registry that is already created in the cluster?
Screen Shot 2022-09-21 at 10 10 40 PM

catherineluse avatar Sep 22 '22 05:09 catherineluse

@catherineluse Yeah - (2) does support credentials - I don't know if there is a standard way to pass this onto the Helm chart for Rancher charts?

I think to start with, we keep (3) simple and don't add support for credentials. We can add username and password later - as above, we could only use the credentials if there was a standard property equivalent to global.cattle.systemDefaultRegistry that let the image pull secret be specified.

nwmac avatar Sep 22 '22 19:09 nwmac

I don't know if there is a standard way to pass this onto the Helm chart for Rancher charts?

@gaktive If the answer to this question is no, it could require backend work.

Based on reading the Helm documentation about how Helm pulls images from private registries, it looks like all Rancher charts would need updates to the values.yaml in order to support the feature. We would need to create a secret with the credentials and then the secret would need to be referenced in their values.yaml. https://helm.sh/docs/howto/charts_tips_and_tricks/#creating-image-pull-secrets

catherineluse avatar Sep 23 '22 01:09 catherineluse

@nwmac from Catherine's comment below, does Team 2 have an issue tied to helm chart updates?

gaktive avatar Sep 23 '22 17:09 gaktive

As discussed, we can skip for now and ignore cluster-level registries that have auth config.

nwmac avatar Sep 23 '22 17:09 nwmac

Found out a bit more info:

  • The cluster level private registry is only available for Rancher provisioned clusters. It doesn't exist for hosted or imported clusters.
  • If a cluster level private registry has credentials, the credentials will already be available and usable on the clusters as soon as they are provisioned. (The credentials are used by docker or containerd from a local config file, and helm doesn't have to know about them.) So only the private registry URL would be needed for helm chart installs on rancher provisioned clusters, regardless of whether the registry has credentials or not.

This also means that for number 3, the custom registry, we could only expose the url of the custom registry, and then explain or document that if they want to configure credentials for that registry, they could configure the credentials for it in the registry config file in the cluster itself (https://docs.rke2.io/install/containerd_registry_configuration/#registries-configuration-file).

catherineluse avatar Sep 23 '22 23:09 catherineluse

QA Template

Root cause

This isn't really a bug fix, but more like a behavior change. Previously, when a user installed a Rancher Helm chart, the images would be pulled from the default global private registry, which is set in the global settings. (By default, that global setting for the default registry is blank, in which case the container runtime uses docker.io to pull the images.) The desired behavior was that if a cluster scoped private registry was configured for a cluster, images would be pulled from that registry by default instead of the one in the global settings.

What was fixed, or what changes have occurred

There are two main changes:

  1. The default source for pulling images for Rancher Helm charts has been changed from the global scoped one to the cluster scoped one, if a cluster scoped one exists. The global one is now used as a fallback instead of the default.
  2. A new section has been added to the wizard workflow for installing Helm charts. It now allows the user to input a custom registry is used to pull the images for the chart. A tooltip explains which default chart would be used if no input is given. (This plan was simplified from the original radio buttons per discussion with Kenneth)
Screen Shot 2022-10-06 at 5 47 48 PM

Areas or cases that should be tested

  • When installing Helm charts, the cluster registry should be selected by default for pulling the images.
  • When a cluster scoped registry is not available, the global scoped registry is used.
  • When installing Helm charts, users can enter a custom registry to use when pulling images for that specific chart.

In more detail:

  1. Provision two downstream RKE2 clusters, one with a private registry in the cluster config and one without.
  2. In the cluster that has a registry, install a helm chart with the default settings. (I used Rancher alerting drivers) Confirm that the cluster registry from the cluster config is used in the values.yaml under values.global.cattle.systemDefaultRegistry.
  3. In the cluster that does not have a registry, install a helm chart with the default settings. The values.global.cattle.systemDefaultRegistry should be an empty string because the global default registry in the settings is also an empty string.
  4. In the global settings of Rancher, change system-default-registry to a different string. It can be any string.
  5. In the cluster that does not have a registry, install or upgrade a chart. Confirm that the values.global.cattle.systemDefaultRegistry now reflects the new default registry from the global settings.
  6. In any cluster, install or upgrade a chart. In the first page of the helm chart install workflow, select the new checkbox that says "Use Custom Container Registry for Rancher system images"
  7. Enter a custom registry
  8. Confirm that the new registry is in the values.global.cattle.systemDefaultRegistry in the values.yaml

What areas could experience regressions?

A misconfigured private registry could cause Helm charts to fail to install. Rancher and non-Rancher Helm charts should be able to be installed successfully.

Are the repro steps accurate/minimal?

N/A - not really a bug.

catherineluse avatar Oct 03 '22 22:10 catherineluse

My PR can't be fully tested until Team 2 confirms the YAML format for configuring the cluster scoped registry. This is because when a cluster is provisioned using a cluster scoped registry, the cluster fails to provision if the global scoped registry is invalid. We wouldn't want to use the cluster scoped registry when installing a helm chart if that registry is configured incorrectly in the cluster YAML. If have opened an issue with more details here https://github.com/rancher/rancher/issues/39338

catherineluse avatar Oct 07 '22 21:10 catherineluse

I've updated the draft PR and tweaked things around a bit

I've flipped the logic of the registry input field. It now always shows the user what setting will be applied (helpful just for their info, determining system vs cluster register, upgrade scenario, etc)

Additionally

  • Show the registry controls if
    • the required values exist in the chart (we don't need to set them if the chart isn't rancher based)
  • Show a value for the registry input field if
    • there is an an existing values (aka upgrade of an app that was installed using a custom or cluster registry)
    • there is a cluster or system registry
  • Apply the value in the registry input field if
    • it differs from the global registry (the system registry is basically a blank entry)

richard-cox avatar Oct 11 '22 17:10 richard-cox

tested using v2.7.0-rc8:

  1. Provision two downstream RKE2 clusters, one with a private registry in the cluster config and one without.
  2. In the cluster that has a registry, install a helm chart with the default settings. (I used Rancher alerting drivers) Confirm that the cluster registry from the cluster config is used in the values.yaml under values.global.cattle.systemDefaultRegistry. -- used monitoring and logging -- pass
  3. In the cluster that does not have a registry, install a helm chart with the default settings. The values.global.cattle.systemDefaultRegistry should be an empty string because the global default registry in the settings is also an empty string. -- used monitoring and logging -- pass
  4. In the global settings of Rancher, change system-default-registry to a different string. It can be any string.
  5. In the cluster that does not have a registry, install or upgrade a chart. Confirm that the values.global.cattle.systemDefaultRegistry now reflects the new default registry from the global settings. -- used monitoring and logging -- pass
  6. In any cluster, install or upgrade a chart. In the first page of the helm chart install workflow, select the new checkbox that says "Use Custom Container Registry for Rancher system images"
  7. Enter a custom registry
  8. Confirm that the new registry is in the values.global.cattle.systemDefaultRegistry in the values.yaml -- used monitoring and logging -- pass

also tested clusters with both system-default-registry and a cluster level registry defined to ensure that the correct registry was used. this was not related to charts, but observed this issue during testing: https://github.com/rancher/rancher/issues/39468

slickwarren avatar Nov 01 '22 22:11 slickwarren