Skip to content

Commit

Permalink
OCPBUGS-44655: Fix multiarch validations using MetadataProvider
Browse files Browse the repository at this point in the history 
Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
  • Loading branch information
@jparrill
jparrill committed Nov 26, 2024
1 parent d419048 commit 9f0f11a
Show file tree
Hide file tree
Showing 12 changed files with 398 additions and 86 deletions.
7 changes: 3 additions & 4 deletions cmd/cluster/core/create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,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
}
}
Expand Down Expand Up @@ -1006,7 +1006,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
Expand All @@ -1015,8 +1015,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
}
Expand Down
10 changes: 8 additions & 2 deletions cmd/cluster/core/create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down
65 changes: 57 additions & 8 deletions hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -147,7 +159,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 },
}
Expand Down Expand Up @@ -891,6 +906,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,
},
},
}
registryclient.DeserializeFunc = func(payload []byte) (*manifestlist.DeserializedManifestList, error) {
return &manifestlist.DeserializedManifestList{
ManifestList: manifestlist.ManifestList{
Manifests: manifests,
},
}, nil
}
hostedClusters := []*hyperv1.HostedCluster{
{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1075,7 +1119,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,
Expand Down Expand Up @@ -1902,7 +1951,7 @@ func TestValidateReleaseImage(t *testing.T) {
"image-4.18.0": "4.18.0",
},
},
&fakeimagemetadataprovider.FakeImageMetadataProvider{
&fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{
Result: &dockerv1client.DockerImageConfig{},
},
nil
Expand Down Expand Up @@ -2244,7 +2293,7 @@ func TestIsUpgradeable(t *testing.T) {
"image-4.14": "4.15.0",
},
},
&fakeimagemetadataprovider.FakeImageMetadataProvider{
&fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{
Result: &dockerv1client.DockerImageConfig{},
},
nil
Expand Down Expand Up @@ -2601,7 +2650,7 @@ func TestIsProgressing(t *testing.T) {
"release-1.3": "1.3.0",
},
},
&fakeimagemetadataprovider.FakeImageMetadataProvider{
&fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{
Result: &dockerv1client.DockerImageConfig{},
},
nil
Expand Down Expand Up @@ -3471,7 +3520,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,
}
Expand Down
2 changes: 1 addition & 1 deletion hypershift-operator/controllers/nodepool/secret_janitor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ spec:
NodePoolReconciler: &NodePoolReconciler{
Client: c,
ReleaseProvider: &fakereleaseprovider.FakeReleaseProvider{Version: supportedversion.LatestSupportedVersion.String()},
ImageMetadataProvider: &fakeimagemetadataprovider.FakeImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{Config: &docker10.DockerConfig{
ImageMetadataProvider: &fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{Config: &docker10.DockerConfig{
Labels: map[string]string{},
}}},
},
Expand Down
21 changes: 13 additions & 8 deletions ignition-server/cmd/run_local_ignitionprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(), "", "", "")
Expand Down
10 changes: 7 additions & 3 deletions ignition-server/controllers/local_ignitionprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
`
}
Expand Down
Loading

0 comments on commit 9f0f11a

Please sign in to comment.