Merge pull request #1118 from alvaroaleman/fix-nodepool-create

Fix create nodepool azure command

api/v1alpha1/nodepool_types.go

@@ -428,8 +428,11 @@ type AgentNodePoolPlatform struct {
 }
 
 type AzureNodePoolPlatform struct {
-	VMSize  string `json:"vmsize"`
-	ImageID string `json:"imageID"`
+	VMSize string `json:"vmsize"`
+	// ImageID is the id of the image to boot from. If unset, the default image at the location below will be used:
+	// subscription/$subscriptionID/resourceGroups/$resourceGroupName/providers/Microsoft.Compute/images/rhcos.x86_64.vhd
+	// +optional
+	ImageID string `json:"imageID,omitempty"`
 	// +kubebuilder:default:=120
 	// +kubebuilder:validation:Minimum=16
 	// +optional

cmd/install/assets/hypershift-operator/hypershift.openshift.io_nodepools.yaml

@@ -423,11 +423,13 @@ spec:
                         minimum: 16
                         type: integer
                       imageID:
+                        description: 'ImageID is the id of the image to boot from.
+                          If unset, the default image at the location below will be
+                          used: subscription/$subscriptionID/resourceGroups/$resourceGroupName/providers/Microsoft.Compute/images/rhcos.x86_64.vhd'
                         type: string
                       vmsize:
                         type: string
                     required:
-                    - imageID
                     - vmsize
                     type: object
                   ibmcloud:

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
 }
 

docs/content/reference/api.md

@@ -1521,6 +1521,9 @@ string
 </em>
 </td>
 <td>
+<em>(Optional)</em>
+<p>ImageID is the id of the image to boot from. If unset, the default image at the location below will be used:
+subscription/$subscriptionID/resourceGroups/$resourceGroupName/providers/Microsoft.Compute/images/rhcos.x86_64.vhd</p>
 </td>
 </tr>
 <tr>

hack/app-sre/saas_template.yaml

@@ -22169,11 +22169,13 @@ objects:
                           minimum: 16
                           type: integer
                         imageID:
+                          description: 'ImageID is the id of the image to boot from.
+                            If unset, the default image at the location below will
+                            be used: subscription/$subscriptionID/resourceGroups/$resourceGroupName/providers/Microsoft.Compute/images/rhcos.x86_64.vhd'
                           type: string
                         vmsize:
                           type: string
                       required:
-                      - imageID
                       - vmsize
                       type: object
                     ibmcloud:

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(bootImage(hcluster, nodePool))},
 		OSDisk: capiazure.OSDisk{
 			DiskSizeGB: utilpointer.Int32Ptr(nodePool.Spec.Platform.Azure.DiskSizeGB),
 		},
@@ -49,3 +50,10 @@ func generateSSHPubkey() (string, error) {
 
 	return base64.StdEncoding.EncodeToString(ssh.MarshalAuthorizedKey(publicRsaKey)), nil
 }
+
+func bootImage(hcluster *hyperv1.HostedCluster, nodepool *hyperv1.NodePool) string {
+	if nodepool.Spec.Platform.Azure.ImageID != "" {
+		return nodepool.Spec.Platform.Azure.ImageID
+	}
+	return fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/images/rhcos.x86_64.vhd", hcluster.Spec.Platform.Azure.SubscriptionID, hcluster.Spec.Platform.Azure.ResourceGroupName)
+}

hypershift-operator/controllers/nodepool/azure_test.go

@@ -0,0 +1,42 @@
+package nodepool
+
+import (
+	"testing"
+
+	hyperv1 "github.com/openshift/hypershift/api/v1alpha1"
+)
+
+func TestBootImage(t *testing.T) {
+	testCases := []struct {
+		name     string
+		hcluster *hyperv1.HostedCluster
+		nodepool *hyperv1.NodePool
+		expected string
+	}{
+		{
+			name: "Nodepool has image set, it is being used",
+			nodepool: &hyperv1.NodePool{Spec: hyperv1.NodePoolSpec{Platform: hyperv1.NodePoolPlatform{Azure: &hyperv1.AzureNodePoolPlatform{
+				ImageID: "nodepool-image",
+			}}}},
+			expected: "nodepool-image",
+		},
+		{
+			name: "Default bootimage is used",
+			hcluster: &hyperv1.HostedCluster{Spec: hyperv1.HostedClusterSpec{Platform: hyperv1.PlatformSpec{Azure: &hyperv1.AzurePlatformSpec{
+				SubscriptionID:    "123-123",
+				ResourceGroupName: "rg-name",
+			}}}},
+			nodepool: &hyperv1.NodePool{Spec: hyperv1.NodePoolSpec{Platform: hyperv1.NodePoolPlatform{Azure: &hyperv1.AzureNodePoolPlatform{}}}},
+			expected: "/subscriptions/123-123/resourceGroups/rg-name/providers/Microsoft.Compute/images/rhcos.x86_64.vhd",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := bootImage(tc.hcluster, tc.nodepool)
+			if result != tc.expected {
+				t.Errorf("expected %s, got %s", tc.expected, result)
+			}
+		})
+	}
+}