diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 32392ee9e1..5f7b40090a 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -3676,31 +3676,19 @@ func (r *HostedControlPlaneReconciler) reconcileClusterVersionOperator(ctx conte if err := r.Get(ctx, client.ObjectKeyFromObject(pullSecret), pullSecret); err != nil { return fmt.Errorf("failed to get pull secret for namespace %s: %w", hcp.Namespace, err) } - - cpRef, err := registryclient.GetCorrectArchImage(ctx, "cluster-version-operator", p.ControlPlaneImage, pullSecret.Data[corev1.DockerConfigJsonKey], r.ImageMetadataProvider) + pullSecretBytes := pullSecret.Data[corev1.DockerConfigJsonKey] + _, controlPlaneReleaseImageRef, err := r.ImageMetadataProvider.GetDigest(ctx, p.ControlPlaneImage, pullSecretBytes) if err != nil { - return fmt.Errorf("failed to parse control plane release image %s: %w", cpRef, err) + return fmt.Errorf("failed to get control plane release image digest %s: %w", p.ControlPlaneImage, err) } - - _, cpReleaseImageRef, err := r.ImageMetadataProvider.GetDigest(ctx, cpRef, pullSecret.Data[corev1.DockerConfigJsonKey]) - if err != nil { - return fmt.Errorf("failed to get control plane release image digest %s: %w", cpRef, err) - } - - controlPlaneReleaseImage = fmt.Sprintf("%s/%s/%s", cpReleaseImageRef.Registry, cpReleaseImageRef.Namespace, cpReleaseImageRef.NameString()) + controlPlaneReleaseImage = controlPlaneReleaseImageRef.String() if p.ControlPlaneImage != hcp.Spec.ReleaseImage { - dpRef, err := registryclient.GetCorrectArchImage(ctx, "cluster-version-operator", hcp.Spec.ReleaseImage, pullSecret.Data[corev1.DockerConfigJsonKey], r.ImageMetadataProvider) + _, dataPlaneReleaseImageRef, err := r.ImageMetadataProvider.GetDigest(ctx, hcp.Spec.ReleaseImage, pullSecretBytes) if err != nil { - return fmt.Errorf("failed to parse data plane release image %s: %w", dpRef, err) + return fmt.Errorf("failed to get release image digest %s: %w", hcp.Spec.ReleaseImage, err) } - - _, dpReleaseImageRef, err := r.ImageMetadataProvider.GetDigest(ctx, dpRef, pullSecret.Data[corev1.DockerConfigJsonKey]) - if err != nil { - return fmt.Errorf("failed to get data plane release image digest %s: %w", dpRef, err) - } - - dataPlaneReleaseImage = fmt.Sprintf("%s/%s/%s", dpReleaseImageRef.Registry, dpReleaseImageRef.Namespace, dpReleaseImageRef.NameString()) + dataPlaneReleaseImage = dataPlaneReleaseImageRef.String() } else { dataPlaneReleaseImage = controlPlaneReleaseImage } diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go b/control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go index d6ea46dc46..b142bd9d94 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go +++ b/control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go @@ -10,7 +10,6 @@ import ( hyperapi "github.com/openshift/hypershift/support/api" "github.com/openshift/hypershift/support/config" component "github.com/openshift/hypershift/support/controlplane-component" - "github.com/openshift/hypershift/support/releaseinfo/registryclient" "github.com/openshift/hypershift/support/util" configv1 "github.com/openshift/api/config/v1" @@ -260,31 +259,26 @@ func discoverCVOReleaseImages(cpContext component.WorkloadContext) (string, stri if err := cpContext.Client.Get(cpContext.Context, client.ObjectKeyFromObject(pullSecret), pullSecret); err != nil { return "", "", fmt.Errorf("failed to get pull secret for namespace %s: %w", cpContext.HCP.Namespace, err) } + pullSecretBytes := pullSecret.Data[corev1.DockerConfigJsonKey] - cpRef, err := registryclient.GetCorrectArchImage(cpContext.Context, "cluster-version-operator", cpContext.HCP.Spec.ReleaseImage, pullSecret.Data[corev1.DockerConfigJsonKey], cpContext.ImageMetadataProvider) - if err != nil { - return "", "", fmt.Errorf("failed to parse control plane release image %s: %w", cpRef, err) + cpReleaseImage := cpContext.HCP.Spec.ReleaseImage + if cpContext.HCP.Spec.ControlPlaneReleaseImage != nil { + cpReleaseImage = *cpContext.HCP.Spec.ControlPlaneReleaseImage } - _, cpReleaseImageRef, err := cpContext.ImageMetadataProvider.GetDigest(cpContext.Context, cpRef, pullSecret.Data[corev1.DockerConfigJsonKey]) + _, controlPlaneReleaseImageRef, err := cpContext.ImageMetadataProvider.GetDigest(cpContext.Context, cpReleaseImage, pullSecretBytes) if err != nil { - return "", "", fmt.Errorf("failed to get control plane release image digest %s: %w", cpRef, err) + return "", "", fmt.Errorf("failed to get control plane release image digest %s: %w", controlPlaneReleaseImageRef, err) } + controlPlaneReleaseImage = controlPlaneReleaseImageRef.String() - controlPlaneReleaseImage = fmt.Sprintf("%s/%s/%s", cpReleaseImageRef.Registry, cpReleaseImageRef.Namespace, cpReleaseImageRef.NameString()) - - if cpContext.HCP.Spec.ControlPlaneReleaseImage != nil && *cpContext.HCP.Spec.ControlPlaneReleaseImage != cpContext.HCP.Spec.ReleaseImage { - dpRef, err := registryclient.GetCorrectArchImage(cpContext.Context, "cluster-version-operator", cpContext.HCP.Spec.ReleaseImage, pullSecret.Data[corev1.DockerConfigJsonKey], cpContext.ImageMetadataProvider) - if err != nil { - return "", "", fmt.Errorf("failed to parse data plane release image %s: %w", dpRef, err) - } - - _, dpReleaseImageRef, err := cpContext.ImageMetadataProvider.GetDigest(cpContext.Context, dpRef, pullSecret.Data[corev1.DockerConfigJsonKey]) + if cpReleaseImage != cpContext.HCP.Spec.ReleaseImage { + _, dataPlaneReleaseImageRef, err := cpContext.ImageMetadataProvider.GetDigest(cpContext.Context, cpContext.HCP.Spec.ReleaseImage, pullSecret.Data[corev1.DockerConfigJsonKey]) if err != nil { - return "", "", fmt.Errorf("failed to get data plane release image digest %s: %w", dpRef, err) + return "", "", fmt.Errorf("failed to get data plane release image digest %s: %w", cpContext.HCP.Spec.ReleaseImage, err) } - dataPlaneReleaseImage = fmt.Sprintf("%s/%s/%s", dpReleaseImageRef.Registry, dpReleaseImageRef.Namespace, dpReleaseImageRef.NameString()) + dataPlaneReleaseImage = dataPlaneReleaseImageRef.String() } else { dataPlaneReleaseImage = controlPlaneReleaseImage } diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index beedc49b77..5b29b15196 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -1066,6 +1066,10 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques } hcluster.Status.PayloadArch = payloadArch + pullSecretBytes, err := hyperutil.GetPullSecretBytes(ctx, r.Client, hcluster) + if err != nil { + return ctrl.Result{}, err + } releaseImage, err := r.lookupReleaseImage(ctx, hcluster, releaseProvider) if err != nil { @@ -1080,7 +1084,15 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques Message: "HostedCluster is at expected version", Reason: hyperv1.AsExpectedReason, } - progressing, err := isProgressing(hcluster, releaseImage) + refWithDigest := func() (string, error) { + _, ref, err := registryClientImageMetadataProvider.GetDigest(ctx, hcluster.Spec.Release.Image, pullSecretBytes) + if err != nil { + return "", err + } + return ref.String(), nil + } + + progressing, err := isProgressing(hcluster, releaseImage, refWithDigest) if err != nil { condition.Status = metav1.ConditionFalse condition.Message = err.Error() @@ -1172,10 +1184,6 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques } } - pullSecretBytes, err := hyperutil.GetPullSecretBytes(ctx, r.Client, hcluster) - if err != nil { - return ctrl.Result{}, err - } controlPlaneOperatorImage, err := GetControlPlaneOperatorImage(ctx, hcluster, releaseProvider, r.HypershiftOperatorImage, pullSecretBytes) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to get controlPlaneOperatorImage: %w", err) @@ -4239,7 +4247,7 @@ func (r *HostedClusterReconciler) validateReleaseImage(ctx context.Context, hc * return supportedversion.IsValidReleaseVersion(&version, currentVersion, &supportedversion.LatestSupportedVersion, &minSupportedVersion, hc.Spec.Networking.NetworkType, hc.Spec.Platform.Type) } -func isProgressing(hc *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseImage) (bool, error) { +func isProgressing(hc *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseImage, refWithDigest func() (string, error)) (bool, error) { for _, condition := range hc.Status.Conditions { switch string(condition.Type) { case string(hyperv1.SupportedHostedCluster), string(hyperv1.ValidHostedClusterConfiguration), string(hyperv1.ValidReleaseImage), string(hyperv1.ReconciliationActive): @@ -4254,7 +4262,12 @@ func isProgressing(hc *hyperv1.HostedCluster, releaseImage *releaseinfo.ReleaseI } } - if hc.Status.Version == nil || hc.Spec.Release.Image != hc.Status.Version.Desired.Image { + withDigest, err := refWithDigest() + if err != nil { + return false, err + } + + if hc.Status.Version == nil || (hc.Spec.Release.Image != hc.Status.Version.Desired.Image && withDigest != hc.Status.Version.Desired.Image) { // cluster is doing initial rollout or upgrading return true, nil } diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go index 00978eaab5..972bd9af35 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go @@ -2485,10 +2485,11 @@ func TestReconciliationSuccessConditionSetting(t *testing.T) { func TestIsProgressing(t *testing.T) { tests := []struct { - name string - hc *hyperv1.HostedCluster - want bool - wantErr bool + name string + hc *hyperv1.HostedCluster + withDigest string + want bool + wantErr bool }{ { name: "stable at release", @@ -2513,6 +2514,30 @@ func TestIsProgressing(t *testing.T) { want: false, wantErr: false, }, + { + name: "stable at release with digest", + hc: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Release: hyperv1.Release{ + Image: "release-1.2", + }, + PullSecret: corev1.LocalObjectReference{ + Name: "pull-secret", + }, + }, + Status: hyperv1.HostedClusterStatus{ + Version: &hyperv1.ClusterVersionStatus{ + Desired: configv1.Release{ + Image: "release-1.2@sha12345", + Version: "1.2.0", + }, + }, + }, + }, + withDigest: "release-1.2@sha12345", + want: false, + wantErr: false, + }, { name: "cluster is rolling out", hc: &hyperv1.HostedCluster{ @@ -2528,6 +2553,30 @@ func TestIsProgressing(t *testing.T) { want: true, wantErr: false, }, + { + name: "cluster is upgrading with digest", + hc: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Release: hyperv1.Release{ + Image: "release-1.3", + }, + PullSecret: corev1.LocalObjectReference{ + Name: "pull-secret", + }, + }, + Status: hyperv1.HostedClusterStatus{ + Version: &hyperv1.ClusterVersionStatus{ + Desired: configv1.Release{ + Image: "release-1.2@sha12345", + Version: "1.2.0", + }, + }, + }, + }, + withDigest: "release-1.3@sha67890", + want: true, + wantErr: false, + }, { name: "cluster is upgrading", hc: &hyperv1.HostedCluster{ @@ -2679,7 +2728,9 @@ func TestIsProgressing(t *testing.T) { if err != nil { t.Errorf("isProgressing() internal err = %v", err) } - got, err := isProgressing(tt.hc, releaseImage) + got, err := isProgressing(tt.hc, releaseImage, func() (string, error) { + return tt.withDigest, nil + }) if (err != nil) != tt.wantErr { t.Errorf("isProgressing() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/ignition-server/controllers/local_ignitionprovider.go b/ignition-server/controllers/local_ignitionprovider.go index e11a788642..2ef1f7881c 100644 --- a/ignition-server/controllers/local_ignitionprovider.go +++ b/ignition-server/controllers/local_ignitionprovider.go @@ -225,7 +225,7 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, cu return nil, err } - mcoComposedImage := fmt.Sprintf("%s/%s/%s", checkedMcoImage.Registry, checkedMcoImage.Namespace, checkedMcoImage.NameString()) + mcoComposedImage := checkedMcoImage.String() if mcoComposedImage != mcoImage { mcoImage = mcoComposedImage log.Info(fmt.Sprintf("using mirrored %s image %v", component, mcoImage)) @@ -376,7 +376,7 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, cu return err } - ccaComposedImage := fmt.Sprintf("%s/%s/%s", checkedClusterConfigImage.Registry, checkedClusterConfigImage.Namespace, checkedClusterConfigImage.NameString()) + ccaComposedImage := checkedClusterConfigImage.String() if ccaComposedImage != clusterConfigImage { clusterConfigImage = ccaComposedImage log.Info(fmt.Sprintf("using mirrored %s image %v", clusterConfigComponent, ccaComposedImage)) diff --git a/support/util/imagemetadata.go b/support/util/imagemetadata.go index d7e2e6f63d..e39dcdd11e 100644 --- a/support/util/imagemetadata.go +++ b/support/util/imagemetadata.go @@ -163,7 +163,7 @@ func (r *RegistryClientImageMetadataProvider) GetDigest(ctx context.Context, ima // Get the image repo info based the source/mirrors in the ICSPs/IDMSs ref = seekOverride(ctx, r.OpenShiftImageRegistryOverrides, parsedImageRef) - composedRef := fmt.Sprintf("%s/%s/%s", ref.Registry, ref.Namespace, ref.NameString()) + composedRef := ref.String() // If the overridden image name is in the cache, return early if imageDigest, exists := digestCache.Get(composedRef); exists { @@ -236,7 +236,7 @@ func (r *RegistryClientImageMetadataProvider) GetManifest(ctx context.Context, i } } - composedRef := fmt.Sprintf("%s/%s/%s", ref.Registry, ref.Namespace, ref.NameString()) + composedRef := ref.String() digestsManifest, srcDigest, err := getManifest(ctx, composedRef, pullSecret) if err != nil { @@ -266,7 +266,7 @@ func (r *RegistryClientImageMetadataProvider) GetMetadata(ctx context.Context, i // Get the image repo info based the source/mirrors in the ICSPs/IDMSs ref = seekOverride(ctx, r.OpenShiftImageRegistryOverrides, parsedImageRef) - composedRef := fmt.Sprintf("%s/%s/%s", ref.Registry, ref.Namespace, ref.NameString()) + composedRef := ref.String() return getMetadata(ctx, composedRef, pullSecret) } @@ -398,12 +398,12 @@ func GetRegistryOverrides(ctx context.Context, ref reference.DockerImageReferenc // and it will assume that is the Name instead if sourceRef.Namespace == "" { if sourceRef.Name == ref.Namespace { - composedImage := fmt.Sprintf("%s/%s/%s", mirrorRef.Registry, ref.Namespace, ref.NameString()) + composedImage := buildComposedRef(mirrorRef.Registry, ref.Namespace, ref.NameString()) composedRef, err := reference.Parse(composedImage) if err != nil { return nil, false, fmt.Errorf("failed to parse composed image reference (partial match) %q: %w", source, err) } - log.Info("registry override coincidence found (namespace)", "original", fmt.Sprintf("%s/%s/%s", ref.Registry, ref.Namespace, ref.NameString()), "mirror", mirror, "composed", composedRef) + log.Info("registry override coincidence found (namespace)", "original", buildComposedRef(ref.Registry, ref.Namespace, ref.NameString()), "mirror", mirror, "composed", composedRef) return &composedRef, true, nil } } @@ -411,12 +411,12 @@ func GetRegistryOverrides(ctx context.Context, ref reference.DockerImageReferenc if ref.Namespace == sourceRef.Namespace && ref.Name == sourceRef.Name { mirrorRef.ID = ref.ID mirrorRef.Tag = ref.Tag - composedImage := fmt.Sprintf("%s/%s/%s", mirrorRef.Registry, mirrorRef.Namespace, mirrorRef.NameString()) + composedImage := buildComposedRef(mirrorRef.Registry, mirrorRef.Namespace, mirrorRef.NameString()) composedRef, err := reference.Parse(composedImage) if err != nil { return nil, false, fmt.Errorf("failed to parse composed image reference (exact match) %q: %w", source, err) } - log.Info("registry override coincidence found (exact match)", "original", fmt.Sprintf("%s/%s/%s", ref.Registry, ref.Namespace, ref.NameString()), "mirror", mirror, "composed", composedRef) + log.Info("registry override coincidence found (exact match)", "original", buildComposedRef(ref.Registry, ref.Namespace, ref.NameString()), "mirror", mirror, "composed", composedRef) return &composedRef, true, nil } @@ -465,3 +465,9 @@ func seekOverride(ctx context.Context, openshiftImageRegistryOverrides map[strin } return &parsedImageReference } + +// buildComposedRef creates a docker image pull reference given its +// separate components +func buildComposedRef(registry, namespace, name string) string { + return fmt.Sprintf("%s/%s/%s", registry, namespace, name) +} diff --git a/support/util/imagemetadata_test.go b/support/util/imagemetadata_test.go index 3452fa76cb..3e00da65f8 100644 --- a/support/util/imagemetadata_test.go +++ b/support/util/imagemetadata_test.go @@ -152,6 +152,7 @@ func TestGetManifest(t *testing.T) { }) } } + func TestGetDigest(t *testing.T) { ctx := context.TODO() pullSecret := []byte("{}") @@ -166,7 +167,7 @@ func TestGetDigest(t *testing.T) { }{ { name: "if failed to parse image reference", - imageRef: "invalid-image-ref", + imageRef: "::invalid-image-ref", pullSecret: pullSecret, expectedErr: true, },