diff --git a/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/k8s-1.31.yaml.template b/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/k8s-1.31.yaml.template index 229f2100d98a7..5103c4c878de1 100644 --- a/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/k8s-1.31.yaml.template +++ b/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/k8s-1.31.yaml.template @@ -195,7 +195,7 @@ metadata: name: cloud-controller-manager namespace: kube-system spec: - replicas: 1 + replicas: {{ ControlPlaneControllerReplicas false }} selector: matchLabels: component: cloud-controller-manager diff --git a/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/kustomization.yaml b/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/kustomization.yaml index 0f1d70e45b55e..cf27ef103eecb 100644 --- a/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/kustomization.yaml +++ b/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/kustomization.yaml @@ -32,6 +32,16 @@ patches: - op: replace path: /spec/template/spec/containers/0/resources/requests/cpu value: '{{ or .ExternalCloudControllerManager.CPURequest "100m" }}' + # Two replicas on a highly available control plane, one otherwise. CCM owns + # cloud-node-lifecycle, which deletes the Node objects of deleted VMs. A single replica can + # be scheduled onto the node a rolling update is about to replace, and once that VM is gone + # nothing is left to delete the stale Node object, so its orphaned pods keep the cluster + # from validating. Only one replica without an HA control plane, because the pods are + # hostNetwork and would otherwise fight over the secure port. The chart emits replicas + # unquoted, so this is patched here and unquoted again by regenerate.sh. + - op: replace + path: /spec/replicas + value: '{{ ControlPlaneControllerReplicas false }}' # kOps schedules the CCM on control-plane nodes that may carry arbitrary taints; tolerate all. - op: replace path: /spec/template/spec/tolerations diff --git a/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/regenerate.sh b/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/regenerate.sh index dad0bb873140c..6901c8dee976a 100755 --- a/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/regenerate.sh +++ b/upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/regenerate.sh @@ -19,5 +19,9 @@ set -euo pipefail cd "$(dirname "$0")" -kustomize build --enable-helm . > k8s-1.31.yaml.template +# replicas is a Go template expression, which kustomize can only carry as a quoted string. +# Unquote it so the rendered manifest has an integer. +kustomize build --enable-helm . \ + | sed "s/^\( replicas: \)'\(.*\)'$/\1\2/" \ + > k8s-1.31.yaml.template echo "Wrote k8s-1.31.yaml.template" diff --git a/upup/pkg/fi/cloudup/azure/status.go b/upup/pkg/fi/cloudup/azure/status.go index 356feafda7484..a226ed4abf3c8 100644 --- a/upup/pkg/fi/cloudup/azure/status.go +++ b/upup/pkg/fi/cloudup/azure/status.go @@ -180,9 +180,13 @@ func (c *azureCloudImplementation) buildCloudInstanceGroup( for _, vm := range vms { // TODO(kenji): Ignore an instance that is being terminated. - // TODO(kenji): Set the status properly so that kops can - // tell whether a VM is up-to-date or not. + // kOps uses a Manual scale set upgrade policy, so latestModelApplied remains false until a + // rolling update replaces the VM. Missing values are treated as up-to-date to prevent + // incomplete Azure data from triggering a rolling update. status := cloudinstances.CloudInstanceStatusUpToDate + if vm.Properties != nil && vm.Properties.LatestModelApplied != nil && !*vm.Properties.LatestModelApplied { + status = cloudinstances.CloudInstanceStatusNeedsUpdate + } _, err := cg.NewCloudInstance(*vm.Name, status, nodeMap[*vm.Name]) if err != nil { return nil, fmt.Errorf("error creating cloud instance group member: %s", err) diff --git a/upup/pkg/fi/cloudup/azure/status_test.go b/upup/pkg/fi/cloudup/azure/status_test.go index ff079e75fac22..c07d0590c2eed 100644 --- a/upup/pkg/fi/cloudup/azure/status_test.go +++ b/upup/pkg/fi/cloudup/azure/status_test.go @@ -317,3 +317,120 @@ func TestGetCloudGroups(t *testing.T) { t.Fatalf("expected min size %d, but got %d", e, a) } } + +func TestGetCloudGroupsNeedsUpdate(t *testing.T) { + const ( + clusterName = "my-cluster" + + nodeIG = "nodes" + nodeVMSS = "nodes.my-cluster" + nodeVM = "nodes.my-cluster_0" + ) + + testCases := []struct { + name string + // A nil value models Azure omitting all VM properties. + properties *compute.VirtualMachineScaleSetVMProperties + needsUpdate bool + }{ + { + name: "latest model applied", + properties: &compute.VirtualMachineScaleSetVMProperties{ + LatestModelApplied: to.Ptr(true), + }, + }, + { + name: "scale set model changed", + properties: &compute.VirtualMachineScaleSetVMProperties{ + LatestModelApplied: to.Ptr(false), + }, + needsUpdate: true, + }, + { + name: "latest model applied not reported", + properties: &compute.VirtualMachineScaleSetVMProperties{}, + }, + { + name: "no properties reported", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + vmssClient := &mockVMScaleSetsClient{ + vmsses: []*compute.VirtualMachineScaleSet{ + { + Name: to.Ptr(nodeVMSS), + Tags: map[string]*string{ + TagClusterName: to.Ptr(clusterName), + }, + SKU: &compute.SKU{ + Capacity: to.Ptr[int64](1), + }, + }, + }, + } + vmClient := &mockVMScaleSetVMsClient{ + vms: []*compute.VirtualMachineScaleSetVM{ + { + Name: to.Ptr(nodeVM), + Properties: tc.properties, + }, + }, + } + + c := &azureCloudImplementation{ + tags: map[string]string{ + TagClusterName: clusterName, + }, + vmscaleSetsClient: vmssClient, + vmscaleSetVMsClient: vmClient, + } + + cluster := &kops.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + }, + Spec: kops.ClusterSpec{ + CloudProvider: kops.CloudProviderSpec{ + Azure: &kops.AzureSpec{ + ResourceGroupName: "my-rg", + }, + }, + }, + } + + instancegroups := []*kops.InstanceGroup{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: nodeIG, + }, + Spec: kops.InstanceGroupSpec{ + Role: kops.InstanceGroupRoleNode, + }, + }, + } + + groups, err := c.GetCloudGroups(cluster, instancegroups, false /* warnUnmatched */, nil) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + group := groups[nodeIG] + if group == nil { + t.Fatalf("expected group %q, but found none", nodeIG) + } + + needUpdate, ready := 1, 0 + if !tc.needsUpdate { + needUpdate, ready = 0, 1 + } + if a, e := len(group.NeedUpdate), needUpdate; a != e { + t.Errorf("expected %d instance(s) needing update, but found %d", e, a) + } + if a, e := len(group.Ready), ready; a != e { + t.Errorf("expected %d ready instance(s), but found %d", e, a) + } + }) + } +}