Skip to content

Commit

Permalink
Fix create nodepool azure command
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
alvaroaleman committed Mar 9, 2022
1 parent df8d070 commit 37f6899
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 9 deletions.
7 changes: 5 additions & 2 deletions api/v1alpha1/nodepool_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
20 changes: 17 additions & 3 deletions cmd/nodepool/azure/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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)
}
Expand All @@ -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
}

Expand Down
3 changes: 3 additions & 0 deletions docs/content/reference/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 3 additions & 1 deletion hack/app-sre/saas_template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
12 changes: 10 additions & 2 deletions hypershift-operator/controllers/nodepool/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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),
},
Expand All @@ -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)
}
42 changes: 42 additions & 0 deletions hypershift-operator/controllers/nodepool/azure_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}

0 comments on commit 37f6899

Please sign in to comment.