Skip to content

Commit

Permalink
Merge pull request #1826 from bryan-cox/HOSTEDCP-577
Browse files Browse the repository at this point in the history 
Retrieve CAPI/CAPA from release image
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Nov 24, 2022
2 parents 0bc2e8d + 78fa5c3 commit 3262046
Show file tree
Hide file tree
Showing 8 changed files with 142 additions and 84 deletions.
59 changes: 16 additions & 43 deletions hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,7 @@ const (
HostedClusterAnnotation = "hypershift.openshift.io/cluster"
clusterDeletionRequeueDuration = 5 * time.Second

// Image built from https://github.com/openshift/cluster-api/tree/release-1.0
// Upstream canonical image comes from https://console.cloud.google.com/gcr/images/k8s-staging-cluster-api/global/
// us.gcr.io/k8s-artifacts-prod/cluster-api/cluster-api-controller:v1.0.0
imageCAPI = "registry.ci.openshift.org/hypershift/cluster-api:v1.0.0"
ImageStreamCAPI = "cluster-capi-controllers"
ImageStreamAutoscalerImage = "cluster-autoscaler"
ImageStreamClusterMachineApproverImage = "cluster-machine-approver"

Expand Down Expand Up @@ -913,7 +910,7 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques
_, ignitionServerHasHealthzHandler := hyperutil.ImageLabels(controlPlaneOperatorImageMetadata)[ignitionServerHealthzHandlerLabel]
_, controlplaneOperatorManagesIgnitionServer := hyperutil.ImageLabels(controlPlaneOperatorImageMetadata)[controlplaneOperatorManagesIgnitionServerLabel]

p, err := platform.GetPlatform(hcluster, utilitiesImage)
p, err := platform.GetPlatform(hcluster, utilitiesImage, pullSecretBytes)
if err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -1343,7 +1340,7 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques
}

// Reconcile the CAPI manager components
err = r.reconcileCAPIManager(ctx, createOrUpdate, hcluster, hcp)
err = r.reconcileCAPIManager(ctx, createOrUpdate, hcluster, hcp, pullSecretBytes)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to reconcile capi manager: %w", err)
}
Expand All @@ -1358,14 +1355,14 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques
// TODO (alberto): drop this after dropping < 4.11 support.
if _, hasLabel := hyperutil.ImageLabels(controlPlaneOperatorImageMetadata)[controlPlaneOperatorManagesMachineAutoscaler]; !hasLabel {
// Reconcile the autoscaler.
err = r.reconcileAutoscaler(ctx, createOrUpdate, hcluster, hcp, utilitiesImage)
err = r.reconcileAutoscaler(ctx, createOrUpdate, hcluster, hcp, utilitiesImage, pullSecretBytes)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to reconcile autoscaler: %w", err)
}
}
if _, hasLabel := hyperutil.ImageLabels(controlPlaneOperatorImageMetadata)[controlPlaneOperatorManagesMachineApprover]; !hasLabel {
// Reconcile the machine approver.
if err = r.reconcileMachineApprover(ctx, createOrUpdate, hcluster, hcp, utilitiesImage); err != nil {
if err = r.reconcileMachineApprover(ctx, createOrUpdate, hcluster, hcp, utilitiesImage, pullSecretBytes); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to reconcile machine approver: %w", err)
}
}
Expand Down Expand Up @@ -1600,7 +1597,7 @@ func ensureHCPAWSRolesBackwardCompatibility(hc *hyperv1.HostedCluster, hcp *hype
}

// reconcileCAPIManager orchestrates orchestrates of all CAPI manager components.
func (r *HostedClusterReconciler) reconcileCAPIManager(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, hcp *hyperv1.HostedControlPlane) error {
func (r *HostedClusterReconciler) reconcileCAPIManager(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, hcp *hyperv1.HostedControlPlane, pullSecretBytes []byte) error {
controlPlaneNamespace := manifests.HostedControlPlaneNamespace(hcluster.Namespace, hcluster.Name)
err := r.Client.Get(ctx, client.ObjectKeyFromObject(controlPlaneNamespace), controlPlaneNamespace)
if err != nil {
Expand Down Expand Up @@ -1688,7 +1685,10 @@ func (r *HostedClusterReconciler) reconcileCAPIManager(ctx context.Context, crea
}

// Reconcile CAPI manager deployment
capiImage := imageCAPI
capiImage, err := hyperutil.GetPayloadImage(ctx, hcluster, ImageStreamCAPI, pullSecretBytes)
if err != nil {
return fmt.Errorf("failed to retrieve capi image: %w", err)
}
if envImage := os.Getenv(images.CAPIEnvVar); len(envImage) > 0 {
capiImage = envImage
}
Expand Down Expand Up @@ -1933,8 +1933,8 @@ func servicePublishingStrategyByType(hcp *hyperv1.HostedCluster, svcType hyperv1
// reconcileAutoscaler orchestrates reconciliation of autoscaler components using
// both the HostedCluster and the HostedControlPlane which the autoscaler takes
// inputs from.
func (r *HostedClusterReconciler) reconcileAutoscaler(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, hcp *hyperv1.HostedControlPlane, utilitiesImage string) error {
clusterAutoscalerImage, err := r.getPayloadImage(ctx, hcluster, ImageStreamAutoscalerImage)
func (r *HostedClusterReconciler) reconcileAutoscaler(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, hcp *hyperv1.HostedControlPlane, utilitiesImage string, pullSecretBytes []byte) error {
clusterAutoscalerImage, err := hyperutil.GetPayloadImage(ctx, hcluster, ImageStreamAutoscalerImage, pullSecretBytes)
if err != nil {
return fmt.Errorf("failed to get image for machine approver: %w", err)
}
Expand Down Expand Up @@ -2531,7 +2531,7 @@ func reconcileCAPIManagerDeployment(deployment *appsv1.Deployment, hc *hyperv1.H
},
},
},
Command: []string{"/manager"},
Command: []string{"/bin/cluster-api-controller-manager"},
Args: []string{"--namespace", "$(MY_NAMESPACE)",
"--alsologtostderr",
"--v=4",
Expand Down Expand Up @@ -3011,7 +3011,7 @@ func (r *HostedClusterReconciler) delete(ctx context.Context, hc *hyperv1.Hosted
}

// Cleanup Platform specifics.
p, err := platform.GetPlatform(hc, "")
p, err := platform.GetPlatform(hc, "", nil)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -3170,8 +3170,8 @@ func (r *HostedClusterReconciler) reconcileClusterPrometheusRBAC(ctx context.Con
return nil
}

func (r *HostedClusterReconciler) reconcileMachineApprover(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, hcp *hyperv1.HostedControlPlane, utilitiesImage string) error {
machineApproverImage, err := r.getPayloadImage(ctx, hcluster, ImageStreamClusterMachineApproverImage)
func (r *HostedClusterReconciler) reconcileMachineApprover(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, hcp *hyperv1.HostedControlPlane, utilitiesImage string, pullSecretBytes []byte) error {
machineApproverImage, err := hyperutil.GetPayloadImage(ctx, hcluster, ImageStreamClusterMachineApproverImage, pullSecretBytes)
if err != nil {
return fmt.Errorf("failed to get image for machine approver: %w", err)
}
Expand Down Expand Up @@ -4081,33 +4081,6 @@ func validateClusterID(hc *hyperv1.HostedCluster) error {
return nil
}

// getReleaseImage get the releaseInfo releaseImage for a given HC release image reference.
func (r *HostedClusterReconciler) getReleaseImage(ctx context.Context, hc *hyperv1.HostedCluster) (*releaseinfo.ReleaseImage, error) {
var pullSecret corev1.Secret
if err := r.Client.Get(ctx, types.NamespacedName{Namespace: hc.Namespace, Name: hc.Spec.PullSecret.Name}, &pullSecret); err != nil {
return nil, fmt.Errorf("failed to get pull secret: %w", err)
}
pullSecretBytes, ok := pullSecret.Data[corev1.DockerConfigJsonKey]
if !ok {
return nil, fmt.Errorf("expected %s key in pull secret", corev1.DockerConfigJsonKey)
}
return r.ReleaseProvider.Lookup(ctx, hc.Spec.Release.Image, pullSecretBytes)
}

// getPayloadImage get an image from the payload for a particular component.
func (r *HostedClusterReconciler) getPayloadImage(ctx context.Context, hc *hyperv1.HostedCluster, component string) (string, error) {
releaseImage, err := r.getReleaseImage(ctx, hc)
if err != nil {
return "", fmt.Errorf("failed to lookup release image: %w", err)
}

image, exists := releaseImage.ComponentImages()[component]
if !exists {
return "", fmt.Errorf("image does not exist for release: %q", image)
}
return image, nil
}

func (r *HostedClusterReconciler) reconcileServiceAccountSigningKey(ctx context.Context, hc *hyperv1.HostedCluster, targetNamespace string, createOrUpdate upsert.CreateOrUpdateFN) error {
privateBytes, publicBytes, err := r.serviceAccountSigningKeyBytes(ctx, hc)
if err != nil {
Expand Down
18 changes: 17 additions & 1 deletion hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/hypershift/api"
hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
version "github.com/openshift/hypershift/cmd/version"
"github.com/openshift/hypershift/control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests"
platformaws "github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/internal/platform/aws"
"github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/internal/platform/kubevirt"
Expand Down Expand Up @@ -840,7 +841,7 @@ func expectedRules(addRules []rbacv1.PolicyRule) []rbacv1.PolicyRule {
}

func TestHostedClusterWatchesEverythingItCreates(t *testing.T) {

releaseImage, _ := version.LookupDefaultOCPVersion()
hostedClusters := []*hyperv1.HostedCluster{
{
ObjectMeta: metav1.ObjectMeta{Name: "agent"},
Expand All @@ -849,6 +850,9 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) {
Type: hyperv1.AgentPlatform,
Agent: &hyperv1.AgentPlatformSpec{AgentNamespace: "agent-namespace"},
},
Release: hyperv1.Release{
Image: releaseImage.PullSpec,
},
},
Status: hyperv1.HostedClusterStatus{
IgnitionEndpoint: "ign",
Expand All @@ -872,6 +876,9 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) {
},
},
},
Release: hyperv1.Release{
Image: releaseImage.PullSpec,
},
},
},
{
Expand All @@ -880,6 +887,9 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) {
Platform: hyperv1.PlatformSpec{
Type: hyperv1.NonePlatform,
},
Release: hyperv1.Release{
Image: releaseImage.PullSpec,
},
},
},
{
Expand All @@ -889,6 +899,9 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) {
Type: hyperv1.IBMCloudPlatform,
IBMCloud: &hyperv1.IBMCloudPlatformSpec{},
},
Release: hyperv1.Release{
Image: releaseImage.PullSpec,
},
},
},
{
Expand All @@ -897,6 +910,9 @@ func TestHostedClusterWatchesEverythingItCreates(t *testing.T) {
Platform: hyperv1.PlatformSpec{
Type: hyperv1.KubevirtPlatform,
},
Release: hyperv1.Release{
Image: releaseImage.PullSpec,
},
},
},
}
Expand Down
7 changes: 6 additions & 1 deletion hypershift-operator/controllers/hostedcluster/hostedcluster_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
hyperapi "github.com/openshift/hypershift/api"
"github.com/openshift/hypershift/api/util/ipnet"
hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
version "github.com/openshift/hypershift/cmd/version"
fakecapabilities "github.com/openshift/hypershift/support/capabilities/fake"
fakereleaseprovider "github.com/openshift/hypershift/support/releaseinfo/fake"
"github.com/openshift/hypershift/support/thirdparty/library-go/pkg/image/dockerv1client"
Expand All @@ -30,6 +31,7 @@ import (
)

func TestWebhookAllowsHostedClusterReconcilerUpdates(t *testing.T) {
releaseImage, _ := version.LookupDefaultOCPVersion()
t.Parallel()
testCases := []struct {
name string
Expand All @@ -55,6 +57,9 @@ func TestWebhookAllowsHostedClusterReconcilerUpdates(t *testing.T) {
},
},
},
Release: hyperv1.Release{
Image: releaseImage.PullSpec,
},
},
},
additionalObjects: []crclient.Object{
Expand All @@ -64,7 +69,7 @@ func TestWebhookAllowsHostedClusterReconcilerUpdates(t *testing.T) {
},
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{Namespace: "some-ns"},
Data: map[string][]byte{".dockerconfigjson": []byte("something")},
Data: map[string][]byte{".dockerconfigjson": []byte("{\"something\": \"something\"}")},
},
&configv1.Ingress{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}},
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "none-cluster"}},
Expand Down
17 changes: 8 additions & 9 deletions hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,19 @@ import (
)

const (
// Image built from https://github.com/openshift/cluster-api-provider-aws/tree/release-1.1
// Upstream canonical image comes from https://console.cloud.google.com/gcr/images/k8s-artifacts-prod
// us.gcr.io/k8s-artifacts-prod/cluster-api-aws/cluster-api-aws-controller:v1.1.0
imageCAPA = "registry.ci.openshift.org/hypershift/cluster-api-aws-controller:v1.1.0"
ImageStreamCAPA = "aws-cluster-api-controllers"
)

func New(utilitiesImage string) *AWS {
func New(utilitiesImage string, capiProviderImage string) *AWS {
return &AWS{
utilitiesImage: utilitiesImage,
utilitiesImage: utilitiesImage,
capiProviderImage: capiProviderImage,
}
}

type AWS struct {
utilitiesImage string
utilitiesImage string
capiProviderImage string
}

func (p AWS) ReconcileCAPIInfraCR(ctx context.Context, c client.Client, createOrUpdate upsert.CreateOrUpdateFN,
Expand All @@ -63,7 +62,7 @@ func (p AWS) ReconcileCAPIInfraCR(ctx context.Context, c client.Client, createOr
}

func (p AWS) CAPIProviderDeploymentSpec(hcluster *hyperv1.HostedCluster, hcp *hyperv1.HostedControlPlane) (*appsv1.DeploymentSpec, error) {
providerImage := imageCAPA
providerImage := p.capiProviderImage
if envImage := os.Getenv(images.AWSCAPIProviderEnvVar); len(envImage) > 0 {
providerImage = envImage
}
Expand Down Expand Up @@ -161,7 +160,7 @@ func (p AWS) CAPIProviderDeploymentSpec(hcluster *hyperv1.HostedCluster, hcp *hy
Value: "true",
},
},
Command: []string{"/manager"},
Command: []string{"/bin/cluster-api-provider-aws-controller-manager"},
Args: []string{"--namespace", "$(MY_NAMESPACE)",
"--alsologtostderr",
"--v=4",
Expand Down
23 changes: 20 additions & 3 deletions hypershift-operator/controllers/hostedcluster/internal/platform/platform.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@ import (
"github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/internal/platform/none"
"github.com/openshift/hypershift/hypershift-operator/controllers/hostedcluster/internal/platform/powervs"
"github.com/openshift/hypershift/support/upsert"
imgUtil "github.com/openshift/hypershift/support/util"
appsv1 "k8s.io/api/apps/v1"
rbacv1 "k8s.io/api/rbac/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

const (
AWSCAPIProvider = "aws-cluster-api-controllers"
)

var _ Platform = aws.AWS{}
var _ Platform = ibmcloud.IBMCloud{}
var _ Platform = none.None{}
Expand Down Expand Up @@ -57,11 +62,23 @@ type Platform interface {
DeleteCredentials(ctx context.Context, c client.Client, hcluster *hyperv1.HostedCluster, controlPlaneNamespace string) error
}

func GetPlatform(hcluster *hyperv1.HostedCluster, utilitiesImage string) (Platform, error) {
var platform Platform
// GetPlatform gets and initializes the cloud platform the hosted cluster was created on
func GetPlatform(hcluster *hyperv1.HostedCluster, utilitiesImage string, pullSecretBytes []byte) (Platform, error) {
var (
platform Platform
capiImageProvider string
err error
)

switch hcluster.Spec.Platform.Type {
case hyperv1.AWSPlatform:
platform = aws.New(utilitiesImage)
if pullSecretBytes != nil {
capiImageProvider, err = imgUtil.GetPayloadImage(context.TODO(), hcluster, AWSCAPIProvider, pullSecretBytes)
if err != nil {
return nil, fmt.Errorf("failed to retrieve capa image: %w", err)
}
}
platform = aws.New(utilitiesImage, capiImageProvider)
case hyperv1.IBMCloudPlatform:
platform = &ibmcloud.IBMCloud{}
case hyperv1.NonePlatform:
Expand Down
Loading

0 comments on commit 3262046

Please sign in to comment.