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 23a876b6356..986991b6ee0 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 7dc74560b6b..745be613069 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 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..f9df451b9cb 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 026f1c22eea..b737b70b4f2 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 bfaf557fc52..6904658fa3e 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 00000000000..fd0d86cccbc --- /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) + } + }) + } +}