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.
openshift · Mar 8, 2022 · 8c6d4f6 · 8c6d4f6
1 parent df8d070
commit 8c6d4f6
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1515,16 +1515,6 @@ string
 </tr>
 <tr>
 <td>
-<code>imageID</code></br>
-<em>
-string
-</em>
-</td>
-<td>
-</td>
-</tr>
-<tr>
-<td>
 <code>diskSizeGB</code></br>
 <em>
 int32
@@ -1643,6 +1633,16 @@ string
 <td>
 </td>
 </tr>
+<tr>
+<td>
+<code>bootImageID</code></br>
+<em>
+string
+</em>
+</td>
+<td>
+</td>
+</tr>
 </tbody>
 </table>
 ###ClusterAutoscaling { #hypershift.openshift.io/v1alpha1.ClusterAutoscaling }

diff --git 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
@@ -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),
 		},