fix: Don't block PATCH VM if billing extension is not installed (#1155)

There are currently cases where a node can join the cluster successfully
but not have the billing extension installed. This blocks identity
propagation and tags propagation.

We tolerate a missing billing extension since this is currently
possible.

Co-authored-by: Bryce Soghigian <[email protected]>
Co-authored-by: Charlie McBride <[email protected]>

Author: 3 people

pkg/controllers/nodeclaim/inplaceupdate/controller.go

@@ -112,6 +112,7 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClaim *karpv1.NodeClaim)
 	if err != nil {
 		return reconcile.Result{}, client.IgnoreNotFound(err)
 	}
+	log.FromContext(ctx).V(1).Info("successfully saved new in-place update hash", "goalHash", goalHash)
 
 	return reconcile.Result{}, nil
 }

pkg/providers/instance/instance.go

@@ -21,6 +21,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"net/http"
 	"sort"
 	"time"
 
@@ -236,6 +237,15 @@ func (p *DefaultProvider) Update(ctx context.Context, vmName string, update armc
 				nil,
 			)
 			if err != nil {
+				// TODO: This is a bit of a hack based on how this Update function is currently used.
+				// Currently this function will not be called by any callers until a claim has been Registered, which means that the CSE had to have succeeded.
+				// The aksIdentifyingExtensionName is not currently guaranteed to be on the VM though, as Karpenter could have failed over during the initial VM create
+				// after CSE but before aksIdentifyingExtensionName. So, for now, we just ignore NotFound errors for the aksIdentifyingExtensionName.
+				azErr := sdkerrors.IsResponseError(err)
+				if extName == aksIdentifyingExtensionName && azErr != nil && azErr.StatusCode == http.StatusNotFound {
+					log.FromContext(ctx).V(0).Info("extension not found when updating tags", "extensionName", extName, "vmName", vmName)
+					continue
+				}
 				return fmt.Errorf("updating VM extension %q for VM %q: %w", extName, vmName, err)
 			}
 			pollers[extName] = poller

pkg/providers/instance/suite_test.go

@@ -19,6 +19,7 @@ package instance_test
 import (
 	"context"
 	"fmt"
+	"net/http"
 	"strings"
 	"testing"
 	"time"
@@ -41,6 +42,7 @@ import (
 	"sigs.k8s.io/karpenter/pkg/test/v1alpha1"
 	. "sigs.k8s.io/karpenter/pkg/utils/testing"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork"
 	"github.com/Azure/karpenter-provider-azure/pkg/apis"
@@ -570,5 +572,55 @@ var _ = Describe("InstanceProvider", func() {
 				"test-tag":                    lo.ToPtr("test-value"),
 			})
 		})
+
+		It("should ignore NotFound errors for computeAksLinuxBilling extension update", func() {
+			// Ensure that the VM already exists in the fake environment
+			vmName := nodeClaim.Name
+			vm := armcompute.VirtualMachine{
+				ID:   lo.ToPtr(fake.MkVMID(azureEnv.AzureResourceGraphAPI.ResourceGroup, vmName)),
+				Name: lo.ToPtr(vmName),
+				Tags: map[string]*string{
+					"karpenter.azure.com_cluster": lo.ToPtr("test-cluster"),
+				},
+			}
+			// Ensure that the NIC already exists in the fake environment
+			azureEnv.VirtualMachinesAPI.Instances.Store(*vm.ID, vm)
+			nic := armnetwork.Interface{
+				ID:   lo.ToPtr(fake.MakeNetworkInterfaceID(azureEnv.AzureResourceGraphAPI.ResourceGroup, vmName)),
+				Name: lo.ToPtr(vmName),
+				Tags: map[string]*string{
+					"karpenter.azure.com_cluster": lo.ToPtr("test-cluster"),
+				},
+			}
+			azureEnv.NetworkInterfacesAPI.NetworkInterfaces.Store(*nic.ID, nic)
+
+			// Ensure that only one extension exists in the env
+			cseExt := armcompute.VirtualMachineExtension{
+				ID:   lo.ToPtr(fake.MakeVMExtensionID(azureEnv.AzureResourceGraphAPI.ResourceGroup, vmName, "cse-agent-karpenter")),
+				Name: lo.ToPtr("cse-agent-karpenter"),
+				Tags: map[string]*string{
+					"karpenter.azure.com_cluster": lo.ToPtr("test-cluster"),
+				},
+			}
+			azureEnv.VirtualMachineExtensionsAPI.Extensions.Store(*cseExt.ID, cseExt)
+			// TODO: This only works because this extension happens to be first in the list of extensions. If it were second it wouldn't work
+			azureEnv.VirtualMachineExtensionsAPI.VirtualMachineExtensionsUpdateBehavior.BeginError.Set(&azcore.ResponseError{StatusCode: http.StatusNotFound}, fake.MaxCalls(1))
+
+			ExpectApplied(ctx, env.Client, nodeClaim, nodePool, nodeClass)
+
+			// Update the VM tags
+			err := azureEnv.InstanceProvider.Update(ctx, vmName, armcompute.VirtualMachineUpdate{
+				Tags: map[string]*string{
+					"karpenter.azure.com_cluster": lo.ToPtr("test-cluster"),
+					"test-tag":                    lo.ToPtr("test-value"),
+				},
+			})
+			Expect(err).ToNot(HaveOccurred())
+
+			ExpectInstanceResourcesHaveTags(ctx, vmName, azureEnv, map[string]*string{
+				"karpenter.azure.com_cluster": lo.ToPtr("test-cluster"),
+				"test-tag":                    lo.ToPtr("test-value"),
+			})
+		})
 	})
 })