From 37f689960304e4b502c85553591c98dde7ba225b 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/v1alpha1/nodepool_types.go | 7 +++- .../hypershift.openshift.io_nodepools.yaml | 4 +- cmd/nodepool/azure/create.go | 20 +++++++-- docs/content/reference/api.md | 3 ++ hack/app-sre/saas_template.yaml | 4 +- .../controllers/nodepool/azure.go | 12 +++++- .../controllers/nodepool/azure_test.go | 42 +++++++++++++++++++ 7 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 hypershift-operator/controllers/nodepool/azure_test.go diff --git a/api/v1alpha1/nodepool_types.go b/api/v1alpha1/nodepool_types.go index 23a876b635..986991b6ee 100644 --- a/api/v1alpha1/nodepool_types.go +++ b/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 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 7dc74560b6..745be61306 100644 --- a/cmd/install/assets/hypershift-operator/hypershift.openshift.io_nodepools.yaml +++ b/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: diff --git a/cmd/nodepool/azure/create.go b/cmd/nodepool/azure/create.go index f49232c18e..982c34ed68 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 404fb486bb..f9df451b9c 100644 --- a/docs/content/reference/api.md +++ b/docs/content/reference/api.md @@ -1521,6 +1521,9 @@ string +(Optional) +

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

diff --git a/hack/app-sre/saas_template.yaml b/hack/app-sre/saas_template.yaml index 026f1c22ee..b737b70b4f 100644 --- a/hack/app-sre/saas_template.yaml +++ b/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: diff --git a/hypershift-operator/controllers/nodepool/azure.go b/hypershift-operator/controllers/nodepool/azure.go index bfaf557fc5..6904658fa3 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(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) +} diff --git a/hypershift-operator/controllers/nodepool/azure_test.go b/hypershift-operator/controllers/nodepool/azure_test.go new file mode 100644 index 0000000000..fd0d86cccb --- /dev/null +++ b/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) + } + }) + } +}