Sitelet https://github.com/kubernetes/kops/commit/0cc198f76e31afb8bf89aa15046bf8caf7b842da
Skip to content

Commit 0cc198f

Browse files
kubernetes-prow[bot]kubernetes-prow[bot]
authored
Merge pull request #18669 from hakman/azure-vmss-needs-update
azure: detect VMs needing update from latestModelApplied
2 parents 9dbe32f + 4410aaa commit 0cc198f

5 files changed

Lines changed: 139 additions & 4 deletions

File tree

‎upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/k8s-1.31.yaml.template‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ metadata:
195195
name: cloud-controller-manager
196196
namespace: kube-system
197197
spec:
198-
replicas: 1
198+
replicas: {{ ControlPlaneControllerReplicas false }}
199199
selector:
200200
matchLabels:
201201
component: cloud-controller-manager

‎upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/kustomization.yaml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,16 @@ patches:
3232
- op: replace
3333
path: /spec/template/spec/containers/0/resources/requests/cpu
3434
value: '{{ or .ExternalCloudControllerManager.CPURequest "100m" }}'
35+
# Two replicas on a highly available control plane, one otherwise. CCM owns
36+
# cloud-node-lifecycle, which deletes the Node objects of deleted VMs. A single replica can
37+
# be scheduled onto the node a rolling update is about to replace, and once that VM is gone
38+
# nothing is left to delete the stale Node object, so its orphaned pods keep the cluster
39+
# from validating. Only one replica without an HA control plane, because the pods are
40+
# hostNetwork and would otherwise fight over the secure port. The chart emits replicas
41+
# unquoted, so this is patched here and unquoted again by regenerate.sh.
42+
- op: replace
43+
path: /spec/replicas
44+
value: '{{ ControlPlaneControllerReplicas false }}'
3545
# kOps schedules the CCM on control-plane nodes that may carry arbitrary taints; tolerate all.
3646
- op: replace
3747
path: /spec/template/spec/tolerations

‎upup/models/cloudup/resources/addons/azure-cloud-controller.addons.k8s.io/regenerate.sh‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,9 @@
1919
set -euo pipefail
2020
cd "$(dirname "$0")"
2121

22-
kustomize build --enable-helm . > k8s-1.31.yaml.template
22+
# replicas is a Go template expression, which kustomize can only carry as a quoted string.
23+
# Unquote it so the rendered manifest has an integer.
24+
kustomize build --enable-helm . \
25+
| sed "s/^\( replicas: \)'\(.*\)'$/\1\2/" \
26+
> k8s-1.31.yaml.template
2327
echo "Wrote k8s-1.31.yaml.template"

‎upup/pkg/fi/cloudup/azure/status.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,13 @@ func (c *azureCloudImplementation) buildCloudInstanceGroup(
180180
for _, vm := range vms {
181181
// TODO(kenji): Ignore an instance that is being terminated.
182182

183-
// TODO(kenji): Set the status properly so that kops can
184-
// tell whether a VM is up-to-date or not.
183+
// kOps uses a Manual scale set upgrade policy, so latestModelApplied remains false until a
184+
// rolling update replaces the VM. Missing values are treated as up-to-date to prevent
185+
// incomplete Azure data from triggering a rolling update.
185186
status := cloudinstances.CloudInstanceStatusUpToDate
187+
if vm.Properties != nil && vm.Properties.LatestModelApplied != nil && !*vm.Properties.LatestModelApplied {
188+
status = cloudinstances.CloudInstanceStatusNeedsUpdate
189+
}
186190
_, err := cg.NewCloudInstance(*vm.Name, status, nodeMap[*vm.Name])
187191
if err != nil {
188192
return nil, fmt.Errorf("error creating cloud instance group member: %s", err)

‎upup/pkg/fi/cloudup/azure/status_test.go‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,3 +317,120 @@ func TestGetCloudGroups(t *testing.T) {
317317
t.Fatalf("expected min size %d, but got %d", e, a)
318318
}
319319
}
320+
321+
func TestGetCloudGroupsNeedsUpdate(t *testing.T) {
322+
const (
323+
clusterName = "my-cluster"
324+
325+
nodeIG = "nodes"
326+
nodeVMSS = "nodes.my-cluster"
327+
nodeVM = "nodes.my-cluster_0"
328+
)
329+
330+
testCases := []struct {
331+
name string
332+
// A nil value models Azure omitting all VM properties.
333+
properties *compute.VirtualMachineScaleSetVMProperties
334+
needsUpdate bool
335+
}{
336+
{
337+
name: "latest model applied",
338+
properties: &compute.VirtualMachineScaleSetVMProperties{
339+
LatestModelApplied: to.Ptr(true),
340+
},
341+
},
342+
{
343+
name: "scale set model changed",
344+
properties: &compute.VirtualMachineScaleSetVMProperties{
345+
LatestModelApplied: to.Ptr(false),
346+
},
347+
needsUpdate: true,
348+
},
349+
{
350+
name: "latest model applied not reported",
351+
properties: &compute.VirtualMachineScaleSetVMProperties{},
352+
},
353+
{
354+
name: "no properties reported",
355+
},
356+
}
357+
358+
for _, tc := range testCases {
359+
t.Run(tc.name, func(t *testing.T) {
360+
vmssClient := &mockVMScaleSetsClient{
361+
vmsses: []*compute.VirtualMachineScaleSet{
362+
{
363+
Name: to.Ptr(nodeVMSS),
364+
Tags: map[string]*string{
365+
TagClusterName: to.Ptr(clusterName),
366+
},
367+
SKU: &compute.SKU{
368+
Capacity: to.Ptr[int64](1),
369+
},
370+
},
371+
},
372+
}
373+
vmClient := &mockVMScaleSetVMsClient{
374+
vms: []*compute.VirtualMachineScaleSetVM{
375+
{
376+
Name: to.Ptr(nodeVM),
377+
Properties: tc.properties,
378+
},
379+
},
380+
}
381+
382+
c := &azureCloudImplementation{
383+
tags: map[string]string{
384+
TagClusterName: clusterName,
385+
},
386+
vmscaleSetsClient: vmssClient,
387+
vmscaleSetVMsClient: vmClient,
388+
}
389+
390+
cluster := &kops.Cluster{
391+
ObjectMeta: metav1.ObjectMeta{
392+
Name: clusterName,
393+
},
394+
Spec: kops.ClusterSpec{
395+
CloudProvider: kops.CloudProviderSpec{
396+
Azure: &kops.AzureSpec{
397+
ResourceGroupName: "my-rg",
398+
},
399+
},
400+
},
401+
}
402+
403+
instancegroups := []*kops.InstanceGroup{
404+
{
405+
ObjectMeta: metav1.ObjectMeta{
406+
Name: nodeIG,
407+
},
408+
Spec: kops.InstanceGroupSpec{
409+
Role: kops.InstanceGroupRoleNode,
410+
},
411+
},
412+
}
413+
414+
groups, err := c.GetCloudGroups(cluster, instancegroups, false /* warnUnmatched */, nil)
415+
if err != nil {
416+
t.Fatalf("unexpected error: %s", err)
417+
}
418+
419+
group := groups[nodeIG]
420+
if group == nil {
421+
t.Fatalf("expected group %q, but found none", nodeIG)
422+
}
423+
424+
needUpdate, ready := 1, 0
425+
if !tc.needsUpdate {
426+
needUpdate, ready = 0, 1
427+
}
428+
if a, e := len(group.NeedUpdate), needUpdate; a != e {
429+
t.Errorf("expected %d instance(s) needing update, but found %d", e, a)
430+
}
431+
if a, e := len(group.Ready), ready; a != e {
432+
t.Errorf("expected %d ready instance(s), but found %d", e, a)
433+
}
434+
})
435+
}
436+
}

0 commit comments

Comments
 (0)