Merge pull request #2847 from bryan-cox/HOSTEDCP-1046-follow-on

HOSTEDCP-1046, HOSTEDCP-1102: Follow-on Items

control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go

@@ -1102,7 +1102,7 @@ func (r *HostedControlPlaneReconciler) reconcile(ctx context.Context, hostedCont
 
 	// Reconcile Ignition
 	r.Log.Info("Reconciling core machine configs")
-	if err := r.reconcileCoreIgnitionConfig(ctx, hostedControlPlane, releaseImageProvider, createOrUpdate); err != nil {
+	if err := r.reconcileCoreIgnitionConfig(ctx, hostedControlPlane, createOrUpdate); err != nil {
 		return fmt.Errorf("failed to reconcile ignition: %w", err)
 	}
 
@@ -3234,7 +3234,7 @@ func (r *HostedControlPlaneReconciler) reconcileManagedTrustedCABundle(ctx conte
 	return nil
 }
 
-func (r *HostedControlPlaneReconciler) reconcileCoreIgnitionConfig(ctx context.Context, hcp *hyperv1.HostedControlPlane, releaseImageProvider *imageprovider.ReleaseImageProvider, createOrUpdate upsert.CreateOrUpdateFN) error {
+func (r *HostedControlPlaneReconciler) reconcileCoreIgnitionConfig(ctx context.Context, hcp *hyperv1.HostedControlPlane, createOrUpdate upsert.CreateOrUpdateFN) error {
 	sshKey := ""
 	if len(hcp.Spec.SSHKey.Name) > 0 {
 		var sshKeySecret corev1.Secret
@@ -3276,37 +3276,17 @@ func (r *HostedControlPlaneReconciler) reconcileCoreIgnitionConfig(ctx context.C
 		return nil
 	}
 
-	version, err := semver.Parse(releaseImageProvider.Version())
-	if err != nil {
-		return fmt.Errorf("failed to determine release image version: %w", err)
+	// ImageDigestMirrorSet is only applicable for release image versions >= 4.13
+	r.Log.Info("Reconciling ImageDigestMirrorSet")
+	imageDigestMirrorSet := globalconfig.ImageDigestMirrorSet()
+	if err := globalconfig.ReconcileImageDigestMirrors(imageDigestMirrorSet, hcp); err != nil {
+		return fmt.Errorf("failed to reconcile image content policy: %w", err)
 	}
 
-	// ImageDigestMirrorSet is only applicable for release image versions greater than or equal to 4.13
-	// TODO the 'else' branch portion needs to be removed after this ticket is backported to 4.13 - HOSTEDCP-1102
-	if version.Minor >= 13 {
-		r.Log.Info("Reconciling ImageDigestMirrorSet")
-		imageDigestMirrorSet := globalconfig.ImageDigestMirrorSet()
-		if err := globalconfig.ReconcileImageDigestMirrors(imageDigestMirrorSet, hcp); err != nil {
-			return fmt.Errorf("failed to reconcile image content policy: %w", err)
-		}
-
-		if _, err := createOrUpdate(ctx, r, imageContentPolicyIgnitionConfig, func() error {
-			return ignition.ReconcileImageSourceMirrorsIgnitionConfigFromIDMS(imageContentPolicyIgnitionConfig, p.OwnerRef, imageDigestMirrorSet)
-		}); err != nil {
-			return fmt.Errorf("failed to reconcile image content source policy ignition config: %w", err)
-		}
-	} else {
-		r.Log.Info("Reconciling ImageContentSourcePolicy")
-		icsp := globalconfig.ImageContentSourcePolicy()
-		if err := globalconfig.ReconcileImageContentSourcePolicy(icsp, hcp); err != nil {
-			return fmt.Errorf("failed to reconcile image content source policy: %w", err)
-		}
-
-		if _, err := createOrUpdate(ctx, r, imageContentPolicyIgnitionConfig, func() error {
-			return ignition.ReconcileImageSourceMirrorsIgnitionConfigFromICSP(imageContentPolicyIgnitionConfig, p.OwnerRef, icsp)
-		}); err != nil {
-			return fmt.Errorf("failed to reconcile image content source policy ignition config from ICSP: %w", err)
-		}
+	if _, err := createOrUpdate(ctx, r, imageContentPolicyIgnitionConfig, func() error {
+		return ignition.ReconcileImageSourceMirrorsIgnitionConfigFromIDMS(imageContentPolicyIgnitionConfig, p.OwnerRef, imageDigestMirrorSet)
+	}); err != nil {
+		return fmt.Errorf("failed to reconcile image content source policy ignition config: %w", err)
 	}
 
 	return nil

control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go

@@ -31,7 +31,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
-	"github.com/blang/semver"
 	"github.com/go-logr/logr"
 	operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 	prometheusoperatorv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
@@ -293,7 +292,7 @@ func (r *reconciler) Reconcile(ctx context.Context, _ ctrl.Request) (ctrl.Result
 	}
 
 	log.Info("reconciling guest cluster global configuration")
-	if err := r.reconcileConfig(ctx, hcp, releaseImage); err != nil {
+	if err := r.reconcileConfig(ctx, hcp); err != nil {
 		errs = append(errs, fmt.Errorf("failed to reconcile global configuration: %w", err))
 	}
 
@@ -580,7 +579,7 @@ func (r *reconciler) reconcileCRDs(ctx context.Context) error {
 	return errors.NewAggregate(errs)
 }
 
-func (r *reconciler) reconcileConfig(ctx context.Context, hcp *hyperv1.HostedControlPlane, releaseImage *releaseinfo.ReleaseImage) error {
+func (r *reconciler) reconcileConfig(ctx context.Context, hcp *hyperv1.HostedControlPlane) error {
 	var errs []error
 
 	apiServerAddress := hcp.Status.ControlPlaneEndpoint.Host
@@ -647,7 +646,7 @@ func (r *reconciler) reconcileConfig(ctx context.Context, hcp *hyperv1.HostedCon
 		errs = append(errs, fmt.Errorf("failed to reconcile proxy config: %w", err))
 	}
 
-	err := r.reconcileImageContentPolicyType(ctx, hcp, releaseImage)
+	err := r.reconcileImageContentPolicyType(ctx, hcp)
 	if err != nil {
 		errs = append(errs, err)
 	}
@@ -2082,37 +2081,24 @@ func (r *reconciler) reconcileStorage(ctx context.Context, hcp *hyperv1.HostedCo
 	return errs
 }
 
-// reconcileImageContentPolicyType reconciles the image content policy based on the release image version.
-// ImageDigestMirrorSets are used for versions >= 4.13 and ImageContentSourcePolicy for all other versions.
-// TODO the 'else' branch portion needs to be removed after this ticket is backported to 4.13 - HOSTEDCP-1102
-func (r *reconciler) reconcileImageContentPolicyType(ctx context.Context, hcp *hyperv1.HostedControlPlane, releaseImage *releaseinfo.ReleaseImage) error {
+// reconcileImageContentPolicyType deletes any existing ICSP since IDMS should be used for release versions >= 4.13,
+// then reconciles the ImageContentSources into an IDMS instance.
+func (r *reconciler) reconcileImageContentPolicyType(ctx context.Context, hcp *hyperv1.HostedControlPlane) error {
 	icsp := globalconfig.ImageContentSourcePolicy()
-	version, err := semver.Parse(releaseImage.Version())
+
+	// Delete any current ICSP
+	_, err := util.DeleteIfNeeded(ctx, r.client, icsp)
 	if err != nil {
-		return fmt.Errorf("failed to determine release image version: %w", err)
+		return fmt.Errorf("failed to delete image content source policy configuration configmap: %w", err)
 	}
 
-	// ImageDigestMirrorSet is only applicable for release image versions greater than or equal to 4.13
-	if version.Minor >= 13 {
-		// First, delete any current ImageContentSourcePolicy
-		_, err = util.DeleteIfNeeded(ctx, r.client, icsp)
-		if err != nil {
-			return fmt.Errorf("failed to delete image content source policy configuration configmap: %w", err)
-		}
-
-		// Next, reconcile the ImageDigestMirrorSet
-		idms := globalconfig.ImageDigestMirrorSet()
-		if _, err = r.CreateOrUpdate(ctx, r.client, idms, func() error {
-			return globalconfig.ReconcileImageDigestMirrors(idms, hcp)
-		}); err != nil {
-			return fmt.Errorf("failed to reconcile image digest mirror set: %w", err)
-		}
-	} else {
-		if _, err = r.CreateOrUpdate(ctx, r.client, icsp, func() error {
-			return globalconfig.ReconcileImageContentSourcePolicy(icsp, hcp)
-		}); err != nil {
-			return fmt.Errorf("failed to reconcile image content source policy: %w", err)
-		}
+	// Next, reconcile the ImageDigestMirrorSet
+	idms := globalconfig.ImageDigestMirrorSet()
+	if _, err = r.CreateOrUpdate(ctx, r.client, idms, func() error {
+		return globalconfig.ReconcileImageDigestMirrors(idms, hcp)
+	}); err != nil {
+		return fmt.Errorf("failed to reconcile image digest mirror set: %w", err)
 	}
+
 	return nil
 }

docs/content/how-to/disconnected/automatically-initialize-registry-overrides.md

@@ -0,0 +1,23 @@
+# Automatically Initialize RegistryOverrides with Image Content Type Policies
+## General
+The HyperShift Operator (HO) will automatically initialize the control plane operator (CPO) with any image registry override information from any ImageContentSourcePolicy (ICSP) or any ImageDigestMirrorSet (IDMS) instances from an OpenShift management cluster.
+
+!!! note
+
+    OpenShift management clusters do not allow both ICSP and IDMS CR instances.
+    IDMS CRs should be used with OpenShift release image versions 4.13 or higher.
+    ICSPs will be deprecated in future OpenShift release versions.
+
+## Technical Implementation Details
+### General
+In `hypershift-operator/main.go`, the HO will look to see if its management cluster has either ICSP or IDMS capabilities. If so, it will retrieve the image registry information from the appropriate ICSP or IDMS instances and store them in a variable called `imageRegistryOverrides`. This variable is then provided to a custom release image provider called `ProviderWithOpenShiftImageRegistryOverrides`.
+
+### ProviderWithOpenShiftImageRegistryOverrides
+The `Lookup` function for `ProviderWithOpenShiftImageRegistryOverrides` will use the source and mirror information in `imageRegistryOverrides` when attempting to look up release images.
+The `ProviderWithOpenShiftImageRegistryOverrides` is also provided to the `HostedClusterReconciler` as its `ReleaseProvider`. When the `HostedClusterReconciler` is reconciling, it will pass any image registry overrides to the ignition server reconciler and the CPO deployment specification.
+
+### Ignition Server
+The ignition server reconciler forwards this information on to the `ignition-server` container as an environment variable called `OPENSHIFT_IMG_OVERRIDES`. `hypershift/ignition-server/cmd/start.go` retrieves this information for the `ReleaseProvider` for the `TokenSecretReconciler`. An important caveat for the ignition server, it cannot follow the ImageContentSourcePolicy or ImageDigestMirrorSet rules because there is not a runtime running inside the pod to do the transformations. So, it's necessary to do the image URL live translation to the custom registry address.
+
+### Control Plane Operator
+The `HostedClusterReconciler` passes on the image registry override information as an environment variable in the CPO called `OPENSHIFT_IMG_OVERRIDES`. The CPO will check for the existence of this environment variable when it runs. If the variable exists, it's used to build the HostedControlPlaneReconciler's `releaseProvider`.

docs/content/how-to/disconnected/disconnected-workarounds.md

@@ -0,0 +1,78 @@
+# Disconnected workarounds
+
+## Make ImageContentSourcePolicies work using image tags
+The ImageContentSourcePolicies (ICSP) have not exposed the API to handle the parameter `mirror-by-digest-only`, so in order to do that, we need to manually create a MachineConfig change to be applied in all the HostedCluster workers. This change will perform a similar action in the workers like an ICSP.
+
+!!! note
+
+    You don't need to do this with future versions of Openshift because `config.openshift.io/v1` has an exposed API to do this, called `ImageTagMirrorSet`.
+
+!!! note
+
+    This workaround could also be applied in the management cluster first, prior to deploying a HostedCluster from the management cluster.
+
+This is our MachineConfig template:
+
+- `mc-icsp-template.yaml`
+```yaml
+---
+apiVersion: machineconfiguration.openshift.io/v1
+kind: MachineConfig
+metadata:
+  labels:
+    machineconfiguration.openshift.io/role: master
+  name: 99-worker-mirror-by-digest-registries
+spec:
+  config:
+    ignition:
+      version: 3.1.0
+    storage:
+      files:
+      - contents:
+          source: data:text/plain;charset=utf-8;base64,$B64_RAWICSP
+        filesystem: root
+        mode: 420
+        path: /etc/containers/registries.conf.d/99-mirror-by-digest-registries.conf
+```
+
+Basically, we create a file inside of `/etc/containers/registries.conf.d/` called `99-mirror-by-digest-registries.conf` which tells the runtime to use the custom registry instead of the external one.
+
+Also, here we have our final file content:
+
+- `icsp-raw-mc.yaml`
+```ini
+[[registry]]
+  prefix = ""
+  location = "registry.redhat.io/openshift4/ose-kube-rbac-proxy"
+  mirror-by-digest-only = false
+
+[[registry.mirror]]
+location = "registry.ocp-edge-cluster-0.qe.lab.redhat.com:5000/openshift4/ose-kube-rbac-proxy"
+
+[[registry]]
+  prefix = ""
+  location = "quay.io/acm-d"
+  mirror-by-digest-only = false
+
+[[registry.mirror]]
+location = "registry.ocp-edge-cluster-0.qe.lab.redhat.com:5000/acm-d"
+
+[[registry]]
+  prefix = ""
+  location = "quay.io/open-cluster-management/addon-manager"
+  mirror-by-digest-only = false
+
+[[registry.mirror]]
+location = "registry.ocp-edge-cluster-0.qe.lab.redhat.com:5000/open-cluster-management/addon-manager"
+```
+
+Now we just need to mix the things up and apply them into our HostedCluster
+
+```bash
+export B64_RAWICSP=$(cat icsp-raw-mc.yaml | base64)
+envsubst < mc-icsp-template.yaml | oc apply -f -
+```
+
+These two commands will create the MachineConfig change in the Openshift cluster, so eventually the worker nodes will get rebooted.
+
+After applying this change, the worker nodes will be able to consume the mirror when only the tags are involved.

docs/content/how-to/disconnected/image-content-sources.md

@@ -0,0 +1,27 @@
+# Setting up Alternative Image Registries Through ImageContentSources
+## What is ImageContentSources and why do we need it?
+Users can set up alternative image registry information for their guest clusters to use through a field called `ImageContentSources` in a hosted control plane's specification. Alternatively, this field can also be set through the HyperShift CLI by specifying a filepath to a file containing the source and mirrors for the alternative registry information, `--image-content-sources=/path/to/file/with/sources_and_mirrors.yml`.
+
+Here is an example of the expected format for this field, `ImageContentSources`:
+```
+- mirrors:
+    - brew.registry.redhat.io
+  source: registry.redhat.io
+- mirrors:
+    - brew.registry.redhat.io
+  source: registry.xyz.redhat.io
+- mirrors:
+    - brew.registry.redhat.io
+  source: registry-proxy.engineering.redhat.com
+```
+!!! note
+
+    This is also the expected format for the file if you choose to use the HyperShift CLI flag.
+
+## How ImageContentSources are used in the nodes within a NodePool
+`ImageContentSources` are reconciled, through the HostedClusterConfigOperator (HCCO), to either an ImageContentSourcePolicy (ICSP) custom resources (CR) or an ImageDigestMirrorSet (IDMS) CR. The CR is then included in the configuration for the Nodes in a NodePool through the NodePool controller's functions, `reconcile > getConfig > defaultAndValidateConfigManifest`.
+
+!!! important
+
+    ICSPs will be deprecated in future OpenShift releases, likely starting with v4.17. IDMSs are the replacement CR for ICSPs but are only available in OpenShift starting in v4.13.
+    The HCCO will automatically delete any ICSPs previously used in the node configuration setup through the Control Plane Operator (CPO) starting in OpenShift v4.13.

docs/mkdocs.yml

@@ -74,6 +74,10 @@ nav:
     - how-to/azure/create-azure-cluster.md
   - 'Agent':
     - how-to/agent/create-agent-cluster.md
+  - 'Disconnected':
+    - how-to/disconnected/automatically-initialize-registry-overrides.md
+    - how-to/disconnected/image-content-sources.md
+    - how-to/disconnected/disconnected-workarounds.md
   - 'Kubevirt':
     - how-to/kubevirt/create-kubevirt-cluster.md
     - how-to/kubevirt/ingress-and-dns.md