From 8c6d4f67383db6882c927b493c3c826e7bf758d2 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Tue, 8 Mar 2022 12:41:50 -0500 Subject: [PATCH] Fix create nodepool azure command This was never fully implemented. In order to fix it, the bootImageId was moved from the nodepool to the cluster, because it is unique per cluster and never changes. Otherwise if there is no nodepool, we are unable to find it. --- api/fixtures/example.go | 2 +- api/v1alpha1/hostedcluster_types.go | 1 + api/v1alpha1/nodepool_types.go | 3 +-- ...ypershift.openshift.io_hostedclusters.yaml | 3 +++ ...hift.openshift.io_hostedcontrolplanes.yaml | 3 +++ .../hypershift.openshift.io_nodepools.yaml | 3 --- cmd/nodepool/azure/create.go | 20 ++++++++++++++++--- docs/content/reference/api.md | 20 +++++++++---------- hack/app-sre/saas_template.yaml | 9 ++++++--- .../controllers/nodepool/azure.go | 5 +++-- 10 files changed, 45 insertions(+), 24 deletions(-) diff --git a/api/fixtures/example.go b/api/fixtures/example.go index 20721b11990..f9c2e1659b1 100644 --- a/api/fixtures/example.go +++ b/api/fixtures/example.go @@ -349,6 +349,7 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token SubscriptionID: o.Azure.Creds.SubscriptionID, MachineIdentityID: o.Azure.MachineIdentityID, SecurityGroupName: o.Azure.SecurityGroupName, + BootImageID: o.Azure.BootImageID, }, } services = o.getIngressServicePublishingStrategyMapping() @@ -551,7 +552,6 @@ web_identity_token_file = /var/run/secrets/openshift/serviceaccount/token nodePool := defaultNodePool(cluster.Name) nodePool.Spec.Platform.Azure = &hyperv1.AzureNodePoolPlatform{ VMSize: o.Azure.InstanceType, - ImageID: o.Azure.BootImageID, DiskSizeGB: o.Azure.DiskSizeGB, } nodePools = append(nodePools, nodePool) diff --git a/api/v1alpha1/hostedcluster_types.go b/api/v1alpha1/hostedcluster_types.go index 5f828718932..35e74f609ad 100644 --- a/api/v1alpha1/hostedcluster_types.go +++ b/api/v1alpha1/hostedcluster_types.go @@ -645,6 +645,7 @@ type AzurePlatformSpec struct { SubscriptionID string `json:"subscriptionID"` MachineIdentityID string `json:"machineIdentityID"` SecurityGroupName string `json:"securityGroupName"` + BootImageID string `json:"bootImageID"` } // Release represents the metadata for an OCP release payload image. diff --git a/api/v1alpha1/nodepool_types.go b/api/v1alpha1/nodepool_types.go index 23a876b6356..67604c607af 100644 --- a/api/v1alpha1/nodepool_types.go +++ b/api/v1alpha1/nodepool_types.go @@ -428,8 +428,7 @@ type AgentNodePoolPlatform struct { } type AzureNodePoolPlatform struct { - VMSize string `json:"vmsize"` - ImageID string `json:"imageID"` + VMSize string `json:"vmsize"` // +kubebuilder:default:=120 // +kubebuilder:validation:Minimum=16 // +optional diff --git a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedclusters.yaml b/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedclusters.yaml index 2c672d98477..070bdd05e0e 100644 --- a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedclusters.yaml +++ b/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedclusters.yaml @@ -575,6 +575,8 @@ spec: azure: description: Azure defines azure specific settings properties: + bootImageID: + type: string credentials: description: LocalObjectReference contains enough information to let you locate the referenced object inside the same @@ -602,6 +604,7 @@ spec: vnetName: type: string required: + - bootImageID - credentials - location - machineIdentityID diff --git a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedcontrolplanes.yaml b/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedcontrolplanes.yaml index 96d0ba119b3..a687226f065 100644 --- a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedcontrolplanes.yaml +++ b/cmd/install/assets/hypershift-operator/hypershift.openshift.io_hostedcontrolplanes.yaml @@ -492,6 +492,8 @@ spec: azure: description: Azure defines azure specific settings properties: + bootImageID: + type: string credentials: description: LocalObjectReference contains enough information to let you locate the referenced object inside the same @@ -519,6 +521,7 @@ spec: vnetName: type: string required: + - bootImageID - credentials - location - machineIdentityID diff --git a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_nodepools.yaml b/cmd/install/assets/hypershift-operator/hypershift.openshift.io_nodepools.yaml index 7dc74560b6b..85f200a7cac 100644 --- a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_nodepools.yaml +++ b/cmd/install/assets/hypershift-operator/hypershift.openshift.io_nodepools.yaml @@ -422,12 +422,9 @@ spec: format: int32 minimum: 16 type: integer - imageID: - type: string vmsize: type: string required: - - imageID - vmsize type: object ibmcloud: diff --git a/cmd/nodepool/azure/create.go b/cmd/nodepool/azure/create.go index f49232c18e3..982c34ed684 100644 --- a/cmd/nodepool/azure/create.go +++ b/cmd/nodepool/azure/create.go @@ -19,6 +19,12 @@ func NewCreateCommand(coreOpts *core.CreateNodePoolOptions) *cobra.Command { Short: "Creates an Azure nodepool", SilenceUsage: true, } + o := &opts{ + instanceType: "Standard_D4s_v4", + diskSize: 120, + } + cmd.Flags().StringVar(&o.instanceType, "instance-type", o.instanceType, "The instance type to use for the nodepool") + cmd.Flags().Int32Var(&o.diskSize, "root-disk-size", o.diskSize, "The size of the root disk for machines in the NodePool (minimum 16)") cmd.Run = func(cmd *cobra.Command, args []string) { ctx, cancel := context.WithCancel(context.Background()) @@ -29,7 +35,7 @@ func NewCreateCommand(coreOpts *core.CreateNodePoolOptions) *cobra.Command { cancel() }() - if err := coreOpts.CreateNodePool(ctx, opts{}); err != nil { + if err := coreOpts.CreateNodePool(ctx, o); err != nil { log.Log.Error(err, "Failed to create nodepool") os.Exit(1) } @@ -38,9 +44,17 @@ func NewCreateCommand(coreOpts *core.CreateNodePoolOptions) *cobra.Command { return cmd } -type opts struct{} +type opts struct { + instanceType string + diskSize int32 +} -func (opts) UpdateNodePool(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, client crclient.Client) error { +func (o *opts) UpdateNodePool(ctx context.Context, nodePool *hyperv1.NodePool, hcluster *hyperv1.HostedCluster, client crclient.Client) error { + nodePool.Spec.Platform.Type = hyperv1.AzurePlatform + nodePool.Spec.Platform.Azure = &hyperv1.AzureNodePoolPlatform{ + VMSize: o.instanceType, + DiskSizeGB: o.diskSize, + } return nil } diff --git a/docs/content/reference/api.md b/docs/content/reference/api.md index 404fb486bb6..ec9b52623cf 100644 --- a/docs/content/reference/api.md +++ b/docs/content/reference/api.md @@ -1515,16 +1515,6 @@ string -imageID
- -string - - - - - - - diskSizeGB
int32 @@ -1643,6 +1633,16 @@ string + + +bootImageID
+ +string + + + + + ###ClusterAutoscaling { #hypershift.openshift.io/v1alpha1.ClusterAutoscaling } diff --git a/hack/app-sre/saas_template.yaml b/hack/app-sre/saas_template.yaml index 026f1c22eea..0b5475a67c8 100644 --- a/hack/app-sre/saas_template.yaml +++ b/hack/app-sre/saas_template.yaml @@ -20147,6 +20147,8 @@ objects: azure: description: Azure defines azure specific settings properties: + bootImageID: + type: string credentials: description: LocalObjectReference contains enough information to let you locate the referenced object inside the same @@ -20174,6 +20176,7 @@ objects: vnetName: type: string required: + - bootImageID - credentials - location - machineIdentityID @@ -21221,6 +21224,8 @@ objects: azure: description: Azure defines azure specific settings properties: + bootImageID: + type: string credentials: description: LocalObjectReference contains enough information to let you locate the referenced object inside the same @@ -21248,6 +21253,7 @@ objects: vnetName: type: string required: + - bootImageID - credentials - location - machineIdentityID @@ -22168,12 +22174,9 @@ objects: format: int32 minimum: 16 type: integer - imageID: - type: string vmsize: type: string required: - - imageID - vmsize type: object ibmcloud: diff --git a/hypershift-operator/controllers/nodepool/azure.go b/hypershift-operator/controllers/nodepool/azure.go index bfaf557fc52..4267f6c3f20 100644 --- a/hypershift-operator/controllers/nodepool/azure.go +++ b/hypershift-operator/controllers/nodepool/azure.go @@ -6,10 +6,11 @@ import ( "encoding/base64" "fmt" - hyperv1 "github.com/openshift/hypershift/api/v1alpha1" "golang.org/x/crypto/ssh" utilpointer "k8s.io/utils/pointer" capiazure "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + + hyperv1 "github.com/openshift/hypershift/api/v1alpha1" ) func azureMachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.NodePool, existing capiazure.AzureMachineTemplateSpec) (*capiazure.AzureMachineTemplateSpec, error) { @@ -25,7 +26,7 @@ func azureMachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1 } return &capiazure.AzureMachineTemplateSpec{Template: capiazure.AzureMachineTemplateResource{Spec: capiazure.AzureMachineSpec{ VMSize: nodePool.Spec.Platform.Azure.VMSize, - Image: &capiazure.Image{ID: &nodePool.Spec.Platform.Azure.ImageID}, + Image: &capiazure.Image{ID: utilpointer.String(hcluster.Spec.Platform.Azure.BootImageID)}, OSDisk: capiazure.OSDisk{ DiskSizeGB: utilpointer.Int32Ptr(nodePool.Spec.Platform.Azure.DiskSizeGB), },