Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-4.18] OCPBUGS-46664,OCPBUGS-46665: Fix IPv6 Disconnected HCP deployments #5318

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
OCPBUGS-44655: Fix multiarch validations using MetadataProvider
Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
  • Loading branch information
jparrill committed Dec 19, 2024
commit a44008c10a752422c3ff999e8e08fab2dcbb3e8a
7 changes: 3 additions & 4 deletions cmd/cluster/core/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down
10 changes: 8 additions & 2 deletions cmd/cluster/core/create_test.go
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
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func cvoLabels() map[string]string {

var port int32 = 8443

func ReconcileDeployment(deployment *appsv1.Deployment, ownerRef config.OwnerRef, deploymentConfig config.DeploymentConfig, controlPlaneImage, releaseImage, cliImage, availabilityProberImage, clusterID string, updateService configv1.URL, platformType hyperv1.PlatformType, oauthEnabled, enableCVOManagementClusterMetricsAccess bool) error {
func ReconcileDeployment(deployment *appsv1.Deployment, ownerRef config.OwnerRef, deploymentConfig config.DeploymentConfig, controlPlaneReleaseImage, dataPlaneReleaseImage, cliImage, availabilityProberImage, clusterID string, updateService configv1.URL, platformType hyperv1.PlatformType, oauthEnabled, enableCVOManagementClusterMetricsAccess bool) error {
ownerRef.ApplyTo(deployment)

// preserve existing resource requirements for main CVO container
Expand All @@ -138,11 +138,11 @@ func ReconcileDeployment(deployment *appsv1.Deployment, ownerRef config.OwnerRef
Spec: corev1.PodSpec{
AutomountServiceAccountToken: ptr.To(false),
InitContainers: []corev1.Container{
util.BuildContainer(cvoContainerPrepPayload(), buildCVOContainerPrepPayload(releaseImage, platformType, oauthEnabled)),
util.BuildContainer(cvoContainerPrepPayload(), buildCVOContainerPrepPayload(dataPlaneReleaseImage, platformType, oauthEnabled)),
util.BuildContainer(cvoContainerBootstrap(), buildCVOContainerBootstrap(cliImage, clusterID)),
},
Containers: []corev1.Container{
util.BuildContainer(cvoContainerMain(), buildCVOContainerMain(controlPlaneImage, releaseImage, deployment.Namespace, updateService, enableCVOManagementClusterMetricsAccess)),
util.BuildContainer(cvoContainerMain(), buildCVOContainerMain(controlPlaneReleaseImage, dataPlaneReleaseImage, deployment.Namespace, updateService, enableCVOManagementClusterMetricsAccess)),
},
Volumes: []corev1.Volume{
util.BuildVolume(cvoVolumePayload(), buildCVOVolumePayload),
Expand Down Expand Up @@ -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")
}

Expand Down Expand Up @@ -330,17 +331,17 @@ oc get clusterversion/version &> /dev/null || oc create -f /tmp/clusterversion.y
return fmt.Sprintf(scriptTemplate, clusterID, payloadDir)
}

func buildCVOContainerMain(image, releaseImage, namespace string, updateService configv1.URL, enableCVOManagementClusterMetricsAccess bool) func(c *corev1.Container) {
func buildCVOContainerMain(controlPlaneReleaseImage, dataPlaneReleaseImage, namespace string, updateService configv1.URL, enableCVOManagementClusterMetricsAccess bool) func(c *corev1.Container) {
cpath := func(vol, file string) string {
return path.Join(volumeMounts.Path(cvoContainerMain().Name, vol), file)
}
return func(c *corev1.Container) {
c.Image = image
c.Image = controlPlaneReleaseImage
c.Command = []string{"cluster-version-operator"}
c.Args = []string{
"start",
"--release-image",
releaseImage,
dataPlaneReleaseImage,
"--enable-auto-update=false",
"--kubeconfig",
path.Join(volumeMounts.Path(c.Name, cvoVolumeKubeconfig().Name), kas.KubeconfigKey),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -173,6 +174,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
}
Expand Down Expand Up @@ -3607,9 +3609,47 @@ func (r *HostedControlPlaneReconciler) reconcileClusterVersionOperator(ctx conte
}
}

var (
controlPlaneReleaseImage string
dataPlaneReleaseImage string
)
// 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)
}

cpRef, err := registryclient.GetCorrectArchImage(ctx, "cluster-version-operator", p.ControlPlaneImage, pullSecret.Data[corev1.DockerConfigJsonKey], r.ImageMetadataProvider)
if err != nil {
return fmt.Errorf("failed to parse control plane release image %s: %w", cpRef, 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())

if p.ControlPlaneImage != hcp.Spec.ReleaseImage {
dpRef, err := registryclient.GetCorrectArchImage(ctx, "cluster-version-operator", hcp.Spec.ReleaseImage, pullSecret.Data[corev1.DockerConfigJsonKey], r.ImageMetadataProvider)
if err != nil {
return fmt.Errorf("failed to parse data plane release image %s: %w", dpRef, 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())
} else {
dataPlaneReleaseImage = controlPlaneReleaseImage
}

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)
return cvo.ReconcileDeployment(deployment, p.OwnerRef, p.DeploymentConfig, controlPlaneReleaseImage, dataPlaneReleaseImage, p.CLIImage, p.AvailabilityProberImage, p.ClusterID, hcp.Spec.UpdateService, p.PlatformType, util.HCPOAuthEnabled(hcp), r.EnableCVOManagementClusterMetricsAccess)
}); err != nil {
return fmt.Errorf("failed to reconcile cluster version operator deployment: %w", err)
}
Expand Down Expand Up @@ -3871,7 +3911,7 @@ func (r *HostedControlPlaneReconciler) reconcileOperatorLifecycleManager(ctx con

var getCatalogImagesErr error
olmCatalogImagesOnce.Do(func() {
catalogImages, err = olm.GetCatalogImages(ctx, *hcp, pullSecret.Data[corev1.DockerConfigJsonKey], registryclient.GetListDigest)
catalogImages, err = olm.GetCatalogImages(ctx, *hcp, pullSecret.Data[corev1.DockerConfigJsonKey], registryclient.GetListDigest, r.ImageMetadataProvider)
if err != nil {
getCatalogImagesErr = err
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ import (
"github.com/openshift/hypershift/support/releaseinfo"
fakereleaseprovider "github.com/openshift/hypershift/support/releaseinfo/fake"
"github.com/openshift/hypershift/support/testutil"
"github.com/openshift/hypershift/support/thirdparty/library-go/pkg/image/dockerv1client"
"github.com/openshift/hypershift/support/upsert"
"github.com/openshift/hypershift/support/util"
"github.com/openshift/hypershift/support/util/fakeimagemetadataprovider"
"go.uber.org/zap/zaptest"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -1646,6 +1648,7 @@ func TestControlPlaneComponents(t *testing.T) {
VPC: &hyperv1.PowerVSVPC{},
},
},
ReleaseImage: "quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64",
},
}

Expand All @@ -1661,8 +1664,12 @@ func TestControlPlaneComponents(t *testing.T) {
CreateOrUpdateProviderV2: upsert.NewV2(false),
ReleaseImageProvider: testutil.FakeImageProvider(),
UserReleaseImageProvider: testutil.FakeImageProvider(),
HCP: hcp,
SkipPredicate: true,
ImageMetadataProvider: &fakeimagemetadataprovider.FakeRegistryClientImageMetadataProvider{
Result: &dockerv1client.DockerImageConfig{},
Manifest: fakeimagemetadataprovider.FakeManifest{},
},
HCP: hcp,
SkipPredicate: true,
}
for _, component := range reconciler.components {
fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).
Expand Down Expand Up @@ -1800,5 +1807,14 @@ func componentsFakeDependencies(componentName string, namespace string) []client
fakeComponents = append(fakeComponents, fakeComponentTemplate.DeepCopy())
}

pullSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "pull-secret", Namespace: "hcp-namespace"},
Data: map[string][]byte{
corev1.DockerConfigJsonKey: []byte(`{}`),
},
}

fakeComponents = append(fakeComponents, pullSecret.DeepCopy())

return fakeComponents
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@ func findTagReference(tags []imagev1.TagReference, name string) *imagev1.TagRefe
return nil
}

func GetCatalogImages(ctx context.Context, hcp hyperv1.HostedControlPlane, pullSecret []byte, digestLister registryclient.DigestListerFN) (map[string]string, error) {
func GetCatalogImages(ctx context.Context, hcp hyperv1.HostedControlPlane, pullSecret []byte, digestLister registryclient.DigestListerFN, imageMetadataProvider util.ImageMetadataProvider) (map[string]string, error) {
imageRef := hcp.Spec.ReleaseImage
imageConfig, _, _, err := registryclient.GetMetadata(ctx, imageRef, pullSecret)
imageConfig, _, _, err := imageMetadataProvider.GetMetadata(ctx, imageRef, pullSecret)
if err != nil {
return nil, fmt.Errorf("failed to get image metadata: %w", err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ spec:
metadata:
annotations:
cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: cloud-token
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
app: cloud-controller-manager
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ spec:
template:
metadata:
annotations:
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
app: cloud-controller-manager
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ spec:
template:
metadata:
annotations:
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
app: cloud-controller-manager
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ spec:
template:
metadata:
annotations:
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
hypershift.openshift.io/control-plane-component: cloud-controller-manager-openstack
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ spec:
template:
metadata:
annotations:
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
hypershift.openshift.io/control-plane-component: cloud-controller-manager-powervs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ spec:
template:
metadata:
annotations:
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
app: cluster-autoscaler
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ spec:
metadata:
annotations:
cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: payload,update-payloads
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
app: cluster-version-operator
Expand Down Expand Up @@ -85,6 +85,7 @@ spec:
- name: CLUSTER_PROFILE
value: ibm-cloud-managed
- name: RELEASE_IMAGE
value: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
- name: NODE_NAME
valueFrom:
fieldRef:
Expand Down Expand Up @@ -281,9 +282,10 @@ spec:
annotations:
include.release.openshift.io/ibm-cloud-managed: "true"
release.openshift.io/delete: "true"
EOF
command:
- /bin/bash
image: cluster-version-operator
image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
imagePullPolicy: IfNotPresent
name: prepare-payload
resources:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ spec:
template:
metadata:
annotations:
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
app: etcd
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ spec:
template:
metadata:
annotations:
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
app: hosted-cluster-config-operator
Expand Down Expand Up @@ -89,6 +89,7 @@ spec:
- name: KUBERNETES_VERSION
value: "1.30"
- name: OPERATE_ON_RELEASE_IMAGE
value: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
- name: OPENSHIFT_IMG_OVERRIDES
value: =
image: hosted-cluster-config-operator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ spec:
annotations:
cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: bootstrap-manifests,logs
component.hypershift.openshift.io/config-hash: 19dc307e1d8949e23415eb6680075ec3
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
app: kube-apiserver
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ spec:
annotations:
cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: logs,certs
component.hypershift.openshift.io/config-hash: "1252551421580954"
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
app: kube-controller-manager
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ spec:
annotations:
cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes: cert-work
component.hypershift.openshift.io/config-hash: 022a8a3a
hypershift.openshift.io/release-image: ""
hypershift.openshift.io/release-image: quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64
creationTimestamp: null
labels:
app: kube-scheduler
Expand Down
Loading