OCPBUGS-44655: Fix multiarch validations using MetadataProvider

Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
openshift · Nov 28, 2024 · 8b691d8 · 8b691d8
1 parent 2569f33
commit 8b691d8
Show file tree

Hide file tree

Showing 16 changed files with 400 additions and 65 deletions.
diff --git a/cmd/cluster/core/create.go b/cmd/cluster/core/create.go
@@ -648,7 +648,7 @@ func (opts *RawCreateOptions) Validate(ctx context.Context) (*ValidatedCreateOpt
 		if err != nil {
 			return nil, fmt.Errorf("could not retrieve kube clientset: %w", err)
 		}
-		if err := validateMgmtClusterAndNodePoolCPUArchitectures(ctx, opts, kc); err != nil {
+		if err := validateMgmtClusterAndNodePoolCPUArchitectures(ctx, opts, kc, &hyperutil.RegistryClientImageMetadataProvider{}); err != nil {
 			return nil, err
 		}
 	}
@@ -1012,7 +1012,7 @@ func parseTolerationString(str string) (*corev1.Toleration, error) {
 // validateMgmtClusterAndNodePoolCPUArchitectures checks if a multi-arch release image or release stream was provided.
 // If none were provided, checks to make sure the NodePool CPU arch and the management cluster CPU arch match; if they
 // do not, the CLI will return an error since the NodePool will fail to complete during runtime.
-func validateMgmtClusterAndNodePoolCPUArchitectures(ctx context.Context, opts *RawCreateOptions, kc kubeclient.Interface) error {
+func validateMgmtClusterAndNodePoolCPUArchitectures(ctx context.Context, opts *RawCreateOptions, kc kubeclient.Interface, imageMetadataProvider hyperutil.ImageMetadataProvider) error {
 	validMultiArchImage := false
 
 	// Check if the release image is multi-arch
@@ -1021,8 +1021,7 @@ func validateMgmtClusterAndNodePoolCPUArchitectures(ctx context.Context, opts *R
 		if err != nil {
 			return fmt.Errorf("failed to read pull secret file: %w", err)
 		}
-
-		validMultiArchImage, err = registryclient.IsMultiArchManifestList(ctx, opts.ReleaseImage, pullSecret)
+		validMultiArchImage, err = registryclient.IsMultiArchManifestList(ctx, opts.ReleaseImage, pullSecret, imageMetadataProvider)
 		if err != nil {
 			return err
 		}

diff --git a/cmd/cluster/core/create_test.go b/cmd/cluster/core/create_test.go
@@ -6,6 +6,8 @@ import (
 
 	. "github.com/onsi/gomega"
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
+	"github.com/openshift/hypershift/support/thirdparty/library-go/pkg/image/dockerv1client"
+	"github.com/openshift/hypershift/support/util/fakeimagemetadataprovider"
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	corev1 "k8s.io/api/core/v1"
@@ -19,11 +21,15 @@ func TestValidateMgmtClusterAndNodePoolCPUArchitectures(t *testing.T) {
 
 	fakeKubeClient := fakekubeclient.NewSimpleClientset()
 	fakeDiscovery, ok := fakeKubeClient.Discovery().(*fakediscovery.FakeDiscovery)
-
 	if !ok {
 		t.Fatalf("failed to convert FakeDiscovery")
 	}
 
+	fakeMetadataProvider := &fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{
+		Result:   &dockerv1client.DockerImageConfig{},
+		Manifest: fakeimagemetadataprovider.FakeManifest{},
+	}
+
 	// if you want to fake a specific version
 	fakeDiscovery.FakedServerVersion = &apiversion.Info{
 		Platform: "linux/amd64",
@@ -80,7 +86,7 @@ func TestValidateMgmtClusterAndNodePoolCPUArchitectures(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			g := NewWithT(t)
-			err := validateMgmtClusterAndNodePoolCPUArchitectures(ctx, tc.opts, fakeKubeClient)
+			err := validateMgmtClusterAndNodePoolCPUArchitectures(ctx, tc.opts, fakeKubeClient, fakeMetadataProvider)
 			if tc.expectError {
 				g.Expect(err).To(HaveOccurred())
 			} else {

diff --git a/control-plane-operator/controllers/hostedcontrolplane/cvo/params.go b/control-plane-operator/controllers/hostedcontrolplane/cvo/params.go
@@ -21,6 +21,7 @@ type CVOParams struct {
 	OwnerRef                config.OwnerRef
 	DeploymentConfig        config.DeploymentConfig
 	PlatformType            hyperv1.PlatformType
+	Component               string
 }
 
 func NewCVOParams(hcp *hyperv1.HostedControlPlane, releaseImageProvider imageprovider.ReleaseImageProvider, setDefaultSecurityContext, enableCVOManagementClusterMetricsAccess bool) *CVOParams {
@@ -32,6 +33,7 @@ func NewCVOParams(hcp *hyperv1.HostedControlPlane, releaseImageProvider imagepro
 		OwnerRef:                config.OwnerRefFrom(hcp),
 		ClusterID:               hcp.Spec.ClusterID,
 		PlatformType:            hcp.Spec.Platform.Type,
+		Component:               "cluster-version-operator",
 	}
 	// fallback to hcp.Spec.ReleaseImage if "cluster-version-operator" image is not available.
 	// This could happen for example in local dev enviroments if the "OPERATE_ON_RELEASE_IMAGE" env variable is not set.

diff --git a/control-plane-operator/controllers/hostedcontrolplane/cvo/reconcile.go b/control-plane-operator/controllers/hostedcontrolplane/cvo/reconcile.go
@@ -300,6 +300,7 @@ func preparePayloadScript(platformType hyperv1.PlatformType, oauthEnabled bool)
 			"    release.openshift.io/delete: \"true\"",
 		)
 	}
+	stmts = append(stmts, "EOF")
 	return strings.Join(stmts, "\n")
 }
 

diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
@@ -5,7 +5,6 @@ import (
 	crand "crypto/rand"
 	"errors"
 	"fmt"
-	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/secretproviderclass"
 	"math/big"
 	"net/http"
 	"os"
@@ -14,6 +13,8 @@ import (
 	"sync"
 	"time"
 
+	"github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/secretproviderclass"
+
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
 	"github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys"
 	azureutil "github.com/Azure/go-autorest/autorest/azure"
@@ -175,6 +176,7 @@ type HostedControlPlaneReconciler struct {
 	awsSession                              *session.Session
 	reconcileInfrastructureStatus           func(ctx context.Context, hcp *hyperv1.HostedControlPlane) (infra.InfrastructureStatus, error)
 	EnableCVOManagementClusterMetricsAccess bool
+	ImageMetadataProvider                   util.ImageMetadataProvider
 
 	IsCPOV2 bool
 }
@@ -3609,6 +3611,19 @@ func (r *HostedControlPlaneReconciler) reconcileClusterVersionOperator(ctx conte
 		}
 	}
 
+	// The CVO prepare-payload script needs the ReleaseImage digest for disconnected environments
+	pullSecret := common.PullSecret(hcp.Namespace)
+	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)
+	}
+
+	manifest, err := registryclient.GetCorrectArchImage(ctx, p.Component, p.ControlPlaneImage, pullSecret.Data[corev1.DockerConfigJsonKey], r.ImageMetadataProvider)
+	if err != nil {
+		return fmt.Errorf("failed to get manifest for image %s: %w", p.ControlPlaneImage, err)
+	}
+
+	fmt.Printf("\nDEBUG - CVO Image - \nCPOImage: %+v \nReleaseImageDigest: %+v\n", p.ControlPlaneImage, manifest)
+
 	deployment := manifests.ClusterVersionOperatorDeployment(hcp.Namespace)
 	if _, err := createOrUpdate(ctx, r, deployment, func() error {
 		return cvo.ReconcileDeployment(deployment, p.OwnerRef, p.DeploymentConfig, p.ControlPlaneImage, p.ReleaseImage, p.CLIImage, p.AvailabilityProberImage, p.ClusterID, hcp.Spec.UpdateService, p.PlatformType, util.HCPOAuthEnabled(hcp), r.EnableCVOManagementClusterMetricsAccess)

diff --git a/control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go b/control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go
@@ -172,6 +172,7 @@ func preparePayloadScript(platformType hyperv1.PlatformType, oauthEnabled bool)
 			"    release.openshift.io/delete: \"true\"",
 		)
 	}
+	stmts = append(stmts, "EOF")
 	return strings.Join(stmts, "\n")
 }
 

diff --git a/control-plane-operator/main.go b/control-plane-operator/main.go
@@ -379,7 +379,6 @@ func NewStartCommand() *cobra.Command {
 			"token-minter":                   tokenMinterImage,
 			util.CPOImageName:                cpoImage,
 			util.CPPKIOImageName:             cpoImage,
-			"cluster-version-operator":       os.Getenv("OPERATE_ON_RELEASE_IMAGE"),
 		}
 		for name, image := range imageOverrides {
 			componentImages[name] = image
@@ -418,6 +417,10 @@ func NewStartCommand() *cobra.Command {
 			OpenShiftImageRegistryOverrides: imageRegistryOverrides,
 		}
 
+		imageMetaDataProvider := &util.RegistryClientImageMetadataProvider{
+			OpenShiftImageRegistryOverrides: imageRegistryOverrides,
+		}
+
 		defaultIngressDomain := os.Getenv(config.DefaultIngressDomainEnvVar)
 
 		metricsSet, err := metrics.MetricsSetFromEnv()
@@ -442,6 +445,7 @@ func NewStartCommand() *cobra.Command {
 			CertRotationScale:                       certRotationScale,
 			EnableCVOManagementClusterMetricsAccess: enableCVOManagementClusterMetricsAccess,
 			IsCPOV2:                                 isCPOV2,
+			ImageMetadataProvider:                   imageMetaDataProvider,
 		}).SetupWithManager(mgr, upsert.New(enableCIDebugOutput).CreateOrUpdate); err != nil {
 			setupLog.Error(err, "unable to create controller", "controller", "hosted-control-plane")
 			os.Exit(1)

diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go
@@ -10,6 +10,8 @@ import (
 
 	"github.com/openshift/hypershift/cmd/util"
 
+	"github.com/docker/distribution"
+	"github.com/docker/distribution/manifest/manifestlist"
 	"github.com/go-logr/logr"
 	"github.com/google/go-cmp/cmp"
 	. "github.com/onsi/gomega"
@@ -30,6 +32,7 @@ import (
 	"github.com/openshift/hypershift/support/config"
 	"github.com/openshift/hypershift/support/releaseinfo"
 	fakereleaseprovider "github.com/openshift/hypershift/support/releaseinfo/fake"
+	"github.com/openshift/hypershift/support/releaseinfo/registryclient"
 	"github.com/openshift/hypershift/support/thirdparty/library-go/pkg/image/dockerv1client"
 	"github.com/openshift/hypershift/support/upsert"
 	hyperutil "github.com/openshift/hypershift/support/util"
@@ -57,8 +60,17 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
-var Now = metav1.NewTime(time.Now())
-var Later = metav1.NewTime(Now.Add(5 * time.Minute))
+var (
+	Now   = metav1.NewTime(time.Now())
+	Later = metav1.NewTime(Now.Add(5 * time.Minute))
+)
+
+const (
+	ManifestListMediaType = "application/vnd.docker.distribution.manifest.list.v2+json"
+	LinuxOS               = "linux"
+	ArchitectureAMD64     = "amd64"
+	ArchitecturePPC64LE   = "ppc64le"
+)
 
 func TestHasBeenAvailable(t *testing.T) {
 	now := time.Now().Truncate(time.Second)
@@ -155,7 +167,10 @@ func TestHasBeenAvailable(t *testing.T) {
 				createOrUpdate:                func(reconcile.Request) upsert.CreateOrUpdateFN { return ctrl.CreateOrUpdate },
 				ManagementClusterCapabilities: &fakecapabilities.FakeSupportNoCapabilities{},
 				ReconcileMetadataProviders: func(ctx context.Context, imgOverrides map[string]string) (releaseinfo.ProviderWithOpenShiftImageRegistryOverrides, hyperutil.ImageMetadataProvider, error) {
-					return &fakereleaseprovider.FakeReleaseProvider{}, &fakeimagemetadataprovider.FakeImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{}}, nil
+					return &fakereleaseprovider.FakeReleaseProvider{}, &fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{
+						Result:   &dockerv1client.DockerImageConfig{},
+						Manifest: fakeimagemetadataprovider.FakeManifest{},
+					}, nil
 				},
 				now: func() metav1.Time { return reconcilerNow },
 			}
@@ -899,6 +914,35 @@ func expectedRules(addRules []rbacv1.PolicyRule) []rbacv1.PolicyRule {
 
 func TestHostedClusterWatchesEverythingItCreates(t *testing.T) {
 	releaseImage, _ := version.LookupDefaultOCPVersion("")
+	manifests := []manifestlist.ManifestDescriptor{
+		{
+			Descriptor: distribution.Descriptor{
+				MediaType: ManifestListMediaType,
+				Digest:    "sha256:70fb4524d21e1b6c08477eb5d1ca2cf282b3270b1d008f70dd7e1cf13d8ba4ce",
+			},
+			Platform: manifestlist.PlatformSpec{
+				Architecture: ArchitectureAMD64,
+				OS:           LinuxOS,
+			},
+		},
+		{
+			Descriptor: distribution.Descriptor{
+				MediaType: ManifestListMediaType,
+				Digest:    "sha256:70fb4524d21e1b6c08477eb5d1ca2cf282b3270b1d008f70dd7e1cf13d8ba4ce",
+			},
+			Platform: manifestlist.PlatformSpec{
+				Architecture: ArchitecturePPC64LE,
+				OS:           LinuxOS,
+			},
+		},
+	}
+	deserializeFunc := func(payload []byte) (*manifestlist.DeserializedManifestList, error) {
+		return &manifestlist.DeserializedManifestList{
+			ManifestList: manifestlist.ManifestList{
+				Manifests: manifests,
+			},
+		}, nil
+	}
 	hostedClusters := []*hyperv1.HostedCluster{
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -1083,7 +1127,12 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) {
 		),
 		createOrUpdate: func(reconcile.Request) upsert.CreateOrUpdateFN { return ctrl.CreateOrUpdate },
 		ReconcileMetadataProviders: func(ctx context.Context, imgOverrides map[string]string) (releaseinfo.ProviderWithOpenShiftImageRegistryOverrides, hyperutil.ImageMetadataProvider, error) {
-			return &fakereleaseprovider.FakeReleaseProvider{}, &fakeimagemetadataprovider.FakeImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{}}, nil
+			return &fakereleaseprovider.FakeReleaseProvider{},
+				&fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{
+					MediaType: ManifestListMediaType,
+					Result:    &dockerv1client.DockerImageConfig{},
+					Manifest:  fakeimagemetadataprovider.FakeManifest{},
+				}, nil
 		},
 		EnableEtcdRecovery: true,
 		now:                metav1.Now,
@@ -1099,7 +1148,8 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) {
 
 	for _, hc := range hostedClusters {
 		t.Run(hc.Name, func(t *testing.T) {
-			_, err := r.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: hc.Namespace, Name: hc.Name}})
+			ctx := context.WithValue(context.Background(), registryclient.DeserializeFuncName, deserializeFunc)
+			_, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: types.NamespacedName{Namespace: hc.Namespace, Name: hc.Name}})
 			if err != nil {
 				t.Fatalf("Reconcile failed: %v", err)
 			}
@@ -1910,7 +1960,7 @@ func TestValidateReleaseImage(t *testing.T) {
 								"image-4.18.0": "4.18.0",
 							},
 						},
-						&fakeimagemetadataprovider.FakeImageMetadataProvider{
+						&fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{
 							Result: &dockerv1client.DockerImageConfig{},
 						},
 						nil
@@ -2252,7 +2302,7 @@ func TestIsUpgradeable(t *testing.T) {
 							"image-4.14":   "4.15.0",
 						},
 					},
-					&fakeimagemetadataprovider.FakeImageMetadataProvider{
+					&fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{
 						Result: &dockerv1client.DockerImageConfig{},
 					},
 					nil
@@ -2609,7 +2659,7 @@ func TestIsProgressing(t *testing.T) {
 							"release-1.3": "1.3.0",
 						},
 					},
-					&fakeimagemetadataprovider.FakeImageMetadataProvider{
+					&fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{
 						Result: &dockerv1client.DockerImageConfig{},
 					},
 					nil
@@ -3535,7 +3585,7 @@ func TestKubevirtETCDEncKey(t *testing.T) {
 				),
 				createOrUpdate: func(reconcile.Request) upsert.CreateOrUpdateFN { return ctrl.CreateOrUpdate },
 				ReconcileMetadataProviders: func(ctx context.Context, imgOverrides map[string]string) (releaseinfo.ProviderWithOpenShiftImageRegistryOverrides, hyperutil.ImageMetadataProvider, error) {
-					return &fakereleaseprovider.FakeReleaseProvider{}, &fakeimagemetadataprovider.FakeImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{}}, nil
+					return &fakereleaseprovider.FakeReleaseProvider{}, &fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{}}, nil
 				},
 				now: metav1.Now,
 			}

diff --git a/hypershift-operator/controllers/nodepool/secret_janitor_test.go b/hypershift-operator/controllers/nodepool/secret_janitor_test.go
@@ -156,7 +156,7 @@ spec:
 			//We need the ReleaseProvider to stay at 4.18 so that the token doesnt get updated when bumping releases,
 			// this protects us from possibly hiding other factors that might be causing the token to be updated
 			ReleaseProvider: &fakereleaseprovider.FakeReleaseProvider{Version: semver.MustParse("4.18.0").String()},
-			ImageMetadataProvider: &fakeimagemetadataprovider.FakeImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{Config: &docker10.DockerConfig{
+			ImageMetadataProvider: &fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{Config: &docker10.DockerConfig{
 				Labels: map[string]string{},
 			}}},
 		},

diff --git a/ignition-server/cmd/run_local_ignitionprovider.go b/ignition-server/cmd/run_local_ignitionprovider.go
@@ -92,15 +92,20 @@ func (o *RunLocalIgnitionProviderOptions) Run(ctx context.Context) error {
 		return fmt.Errorf("unable to create image file cache: %w", err)
 	}
 
+	imageMetaDataProvider := &util.RegistryClientImageMetadataProvider{
+		OpenShiftImageRegistryOverrides: map[string][]string{},
+	}
+
 	p := &controllers.LocalIgnitionProvider{
-		Client:              cl,
-		ReleaseProvider:     &releaseinfo.ProviderWithOpenShiftImageRegistryOverridesDecorator{},
-		CloudProvider:       "",
-		Namespace:           o.Namespace,
-		WorkDir:             o.WorkDir,
-		PreserveOutput:      true,
-		ImageFileCache:      imageFileCache,
-		FeatureGateManifest: o.FeatureGateManifest,
+		Client:                cl,
+		ReleaseProvider:       &releaseinfo.ProviderWithOpenShiftImageRegistryOverridesDecorator{},
+		ImageMetadataProvider: imageMetaDataProvider,
+		CloudProvider:         "",
+		Namespace:             o.Namespace,
+		WorkDir:               o.WorkDir,
+		PreserveOutput:        true,
+		ImageFileCache:        imageFileCache,
+		FeatureGateManifest:   o.FeatureGateManifest,
 	}
 
 	payload, err := p.GetPayload(ctx, o.Image, config.String(), "", "", "")

diff --git a/ignition-server/controllers/local_ignitionprovider.go b/ignition-server/controllers/local_ignitionprovider.go
@@ -72,6 +72,10 @@ type LocalIgnitionProvider struct {
 	// to render the ignition payload.
 	FeatureGateManifest string
 
+	// ImageMetaDataProvider is used to get the image metadata for the images
+	// used in the ignition payload.
+	ImageMetadataProvider *util.RegistryClientImageMetadataProvider
+
 	ImageFileCache *imageFileCache
 
 	lock sync.Mutex
@@ -191,7 +195,7 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, cu
 		return nil, fmt.Errorf("release image does not contain machine-config-operator (images: %v)", imageProvider.ComponentImages())
 	}
 
-	mcoImage, err = registryclient.GetCorrectArchImage(ctx, component, mcoImage, pullSecret)
+	mcoImage, err = registryclient.GetCorrectArchImage(ctx, component, mcoImage, pullSecret, p.ImageMetadataProvider)
 	if err != nil {
 		return nil, err
 	}
@@ -329,7 +333,7 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, cu
 			return fmt.Errorf("release image does not contain $%s (images: %v)", clusterConfigComponent, imageProvider.ComponentImages())
 		}
 
-		clusterConfigImage, err = registryclient.GetCorrectArchImage(ctx, clusterConfigComponent, clusterConfigImage, pullSecret)
+		clusterConfigImage, err = registryclient.GetCorrectArchImage(ctx, clusterConfigComponent, clusterConfigImage, pullSecret, p.ImageMetadataProvider)
 		if err != nil {
 			return err
 		}
@@ -750,7 +754,7 @@ EOF
    --asset-input-dir %[2]s/input \
    --asset-output-dir %[2]s/output \
    --rendered-manifest-files=%[2]s/manifests \
-   --payload-version=%[4]s 
+   --payload-version=%[4]s
 cp %[2]s/manifests/99_feature-gate.yaml %[3]s/99_feature-gate.yaml
 `
 	}
-Original file line number
+Diff line change
@@ Expand Up @@
     			"    release.openshift.io/delete: \"true\"",
     		)
     	}
+    	stmts = append(stmts, "EOF")
     	return strings.Join(stmts, "\n")
     }
@@ Expand Down @@