Skip to content

Commit

Permalink
fix: Use providerID from node obj instead of handcraft one (#173)
Browse files Browse the repository at this point in the history
The node providerID has been updated by cloudprovider after node object
is created. We can always assume the vmID in vmss is 0. We should read
the providerID from the node object instead of handcrafting one with
hardcoded index of 0.
  • Loading branch information
Fei-Guo authored Nov 1, 2024
1 parent 3372fc7 commit b173a75
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 37 deletions.
18 changes: 1 addition & 17 deletions pkg/providers/instance/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,6 @@ func (p *Provider) Create(ctx context.Context, machine *v1alpha5.Machine) (*Inst
return instance, err
}

// getVMSSNodeProviderID generates the provider ID for a virtual machine scale set.
func (p *Provider) getVMSSNodeProviderID(subscriptionID, scaleSetName string) string {
return fmt.Sprintf(
"/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/virtualMachineScaleSets/%s/virtualMachines/0", //vm = 0 as ew have the count always 1
subscriptionID,
strings.ToLower(p.nodeResourceGroup),
scaleSetName,
)
}

func (p *Provider) Get(ctx context.Context, id string) (*Instance, error) {
apName, err := utils.ParseAgentPoolNameFromID(id)
if err != nil {
Expand Down Expand Up @@ -196,11 +186,6 @@ func (p *Provider) Delete(ctx context.Context, id string) error {
}

func (p *Provider) fromAgentPoolToInstance(ctx context.Context, apObj *armcontainerservice.AgentPool) (*Instance, error) {
subID, err := utils.ParseSubIDFromID(lo.FromPtr(apObj.ID))
if err != nil {
return nil, err
}

node, err := p.getNodeByName(ctx, lo.FromPtr(apObj.Name))
if err != nil {
return nil, err
Expand All @@ -209,13 +194,12 @@ func (p *Provider) fromAgentPoolToInstance(ctx context.Context, apObj *armcontai
// node is not found or not ready
return nil, nil
}
tokens := strings.SplitAfter(node.Name, "-vmss") // remove the vm index "0000"
instanceLabels := lo.MapValues(apObj.Properties.NodeLabels, func(k *string, _ string) string {
return lo.FromPtr(k)
})
return &Instance{
Name: apObj.Name,
ID: to.Ptr(fmt.Sprint("azure://", p.getVMSSNodeProviderID(lo.FromPtr(subID), tokens[0]))),
ID: to.Ptr(node.Spec.ProviderID),
Type: apObj.Properties.VMSize,
SubnetID: apObj.Properties.VnetSubnetID,
Tags: apObj.Properties.Tags,
Expand Down
20 changes: 0 additions & 20 deletions pkg/providers/instance/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package instance
import (
"context"
"errors"
"fmt"
"net/http"
"testing"

Expand Down Expand Up @@ -195,11 +194,6 @@ func TestFromAgentPoolToInstance(t *testing.T) {
},
expectedError: errors.New("Fail to get node list"),
},
{
name: "Fail to get instance from agent pool due to malformed id",
mockAgentPool: tests.GetAgentPoolObjWithName("agentpool0", "/subscriptions/resourcegroups/nodeRG/providers/Microsoft.Compute/virtualMachineScaleSets/aks-agentpool0-20562481-vmss", "Standard_NC6s_v3"),
expectedError: errors.New("id does not match the regxp for ParseSubIDFromID"),
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -466,20 +460,6 @@ func TestFromAPListToInstanceFailure(t *testing.T) {
return errors.New("no agentpools found")
},
},
{
name: "Fail to get instance from agent pool list because agentpool subId can't be parsed",
id: "/subscriptions/resourcegroups/nodeRG/providers/Microsoft.Compute/virtualMachineScaleSets/aks-agentpool0-20562481-vmss",
mockAgentPoolList: func(id string) []*armcontainerservice.AgentPool {
ap := tests.GetAgentPoolObjWithName("agentpool0", id, "Standard_NC6s_v3")

return []*armcontainerservice.AgentPool{
&ap,
}
},
expectedError: func(err string) error {
return fmt.Errorf("id does not match the regxp for ParseSubIDFromID %s", err)
},
},
}

for _, tc := range testCases {
Expand Down
3 changes: 3 additions & 0 deletions pkg/tests/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ var (
"kubernetes.azure.com/agentpool": "agentpool0",
},
},
Spec: v1.NodeSpec{
ProviderID: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/nodeRG/providers/Microsoft.Compute/virtualMachineScaleSets/aks-agentpool0-20562481-vmss/virtualMachines/0",
},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{
{
Expand Down

0 comments on commit b173a75

Please sign in to comment.