Merge pull request #10167 from olemarkus/cilium-ondelete

Make it possible to use OnDelete update strategy on addon daemonset

channels/pkg/api/channel.go

@@ -62,6 +62,11 @@ type AddonSpec struct {
 	// version of the software we are packaging.  But we always want to reinstall when we
 	// switch kubernetes versions.
 	Id string `json:"id,omitempty"`
+
+	// NeedsRollingUpdate determines if we should mark nodes as needing an update.
+	// Legal values are control-plane, workers, and all
+	// Empty value means no update needed
+	NeedsRollingUpdate string `json:"needsRollingUpdate,omitempty"`
 }
 
 func (a *Addons) Verify() error {

channels/pkg/channels/addon.go

@@ -18,13 +18,18 @@ package channels
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"net/url"
 
+	"k8s.io/apimachinery/pkg/types"
+
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
 	"k8s.io/kops/channels/pkg/api"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // Addon is a wrapper around a single version of an addon
@@ -144,6 +149,13 @@ func (a *Addon) EnsureUpdated(ctx context.Context, k8sClient kubernetes.Interfac
 		return nil, fmt.Errorf("error applying update from %q: %v", manifestURL, err)
 	}
 
+	if a.Spec.NeedsRollingUpdate != "" {
+		err = a.AddNeedsUpdateLabel(ctx, k8sClient)
+		if err != nil {
+			return nil, fmt.Errorf("error adding needs-update label: %v", err)
+		}
+	}
+
 	channel := a.buildChannel()
 	err = channel.SetInstalledVersion(ctx, k8sClient, a.ChannelVersion())
 	if err != nil {
@@ -152,3 +164,36 @@ func (a *Addon) EnsureUpdated(ctx context.Context, k8sClient kubernetes.Interfac
 
 	return required, nil
 }
+
+func (a *Addon) AddNeedsUpdateLabel(ctx context.Context, k8sClient kubernetes.Interface) error {
+	klog.Infof("addon %v wants to update %v nodes", a.Name, a.Spec.NeedsRollingUpdate)
+	selector := ""
+	switch a.Spec.NeedsRollingUpdate {
+	case "control-plane":
+		selector = "node-role.kubernetes.io/master="
+	case "worker":
+		selector = "node-role.kubernetes.io/node="
+	}
+
+	annotationPatch := &annotationPatch{Metadata: annotationPatchMetadata{Annotations: map[string]string{
+		"kops.k8s.io/needs-update": "",
+	}}}
+	annotationPatchJSON, err := json.Marshal(annotationPatch)
+	if err != nil {
+		return err
+	}
+
+	nodeInterface := k8sClient.CoreV1().Nodes()
+	nodes, err := nodeInterface.List(ctx, metav1.ListOptions{LabelSelector: selector})
+	if err != nil {
+		return err
+	}
+	for _, node := range nodes.Items {
+		_, err = nodeInterface.Patch(ctx, node.Name, types.StrategicMergePatchType, annotationPatchJSON, metav1.PatchOptions{})
+
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}

cmd/kops/get_instances.go

@@ -126,6 +126,7 @@ func RunGetInstances(ctx context.Context, f *util.Factory, out io.Writer, option
 	var cloudInstances []*cloudinstances.CloudInstance
 
 	cloudGroups, err := cloud.GetCloudGroups(cluster, instanceGroups, false, nodeList.Items)
+
 	if err != nil {
 		return err
 	}
@@ -133,6 +134,7 @@ func RunGetInstances(ctx context.Context, f *util.Factory, out io.Writer, option
 	for _, cg := range cloudGroups {
 		cloudInstances = append(cloudInstances, cg.Ready...)
 		cloudInstances = append(cloudInstances, cg.NeedUpdate...)
+		cg.AdjustNeedUpdate()
 	}
 
 	switch options.output {

cmd/kops/rollingupdatecluster.go

@@ -350,7 +350,7 @@ func RunRollingUpdateCluster(ctx context.Context, f *util.Factory, out io.Writer
 		ValidateSuccessDuration: 10 * time.Second,
 	}
 
-	err = d.AdjustNeedUpdate(groups, list)
+	err = d.AdjustNeedUpdate(groups)
 	if err != nil {
 		return err
 	}

pkg/cloudinstances/cloud_instance_group.go

@@ -75,6 +75,29 @@ func (c *CloudInstanceGroup) Status() string {
 	return "NeedsUpdate"
 }
 
+func (group *CloudInstanceGroup) AdjustNeedUpdate() {
+
+	if group.Ready != nil {
+		var newReady []*CloudInstance
+		for _, member := range group.Ready {
+			makeNotReady := false
+			if member.Node != nil && member.Node.Annotations != nil {
+				if _, ok := member.Node.Annotations["kops.k8s.io/needs-update"]; ok {
+					makeNotReady = true
+				}
+			}
+
+			if makeNotReady {
+				group.NeedUpdate = append(group.NeedUpdate, member)
+				member.Status = CloudInstanceStatusNeedsUpdate
+			} else {
+				newReady = append(newReady, member)
+			}
+		}
+		group.Ready = newReady
+	}
+}
+
 // GetNodeMap returns a list of nodes keyed by their external id
 func GetNodeMap(nodes []v1.Node, cluster *kopsapi.Cluster) map[string]*v1.Node {
 	nodeMap := make(map[string]*v1.Node)

pkg/instancegroups/rollingupdate.go

@@ -80,27 +80,9 @@ type RollingUpdateCluster struct {
 }
 
 // AdjustNeedUpdate adjusts the set of instances that need updating, using factors outside those known by the cloud implementation
-func (c *RollingUpdateCluster) AdjustNeedUpdate(groups map[string]*cloudinstances.CloudInstanceGroup, instanceGroups *api.InstanceGroupList) error {
+func (*RollingUpdateCluster) AdjustNeedUpdate(groups map[string]*cloudinstances.CloudInstanceGroup) error {
 	for _, group := range groups {
-		if group.Ready != nil {
+		group.AdjustNeedUpdate()
-			var newReady []*cloudinstances.CloudInstance
-			for _, member := range group.Ready {
-				makeNotReady := false
-				if member.Node != nil && member.Node.Annotations != nil {
-					if _, ok := member.Node.Annotations["kops.k8s.io/needs-update"]; ok {
-						makeNotReady = true
-					}
-				}
-
-				if makeNotReady {
-					group.NeedUpdate = append(group.NeedUpdate, member)
-					member.Status = cloudinstances.CloudInstanceStatusNeedsUpdate
-				} else {
-					newReady = append(newReady, member)
-				}
-			}
-			group.Ready = newReady
-		}
 	}
 	return nil
 }

pkg/instancegroups/rollingupdate_test.go

@@ -736,7 +736,7 @@ func TestAddAnnotatedNodesToNeedsUpdate(t *testing.T) {
 	addNeedsUpdateAnnotation(groups["node-2"], "node-2a")
 	addNeedsUpdateAnnotation(groups["master-1"], "master-1b")
 
-	err := c.AdjustNeedUpdate(groups, &kopsapi.InstanceGroupList{})
+	err := c.AdjustNeedUpdate(groups)
 	assert.NoError(t, err, "AddAnnotatedNodesToGroups")
 
 	assertGroupNeedUpdate(t, groups, "node-1", "node-1a", "node-1b")
@@ -759,7 +759,7 @@ func TestAddAnnotatedNodesToNeedsUpdateCloudonly(t *testing.T) {
 	c.CloudOnly = true
 	c.ClusterValidator = &assertNotCalledClusterValidator{T: t}
 
-	err := c.AdjustNeedUpdate(groups, &kopsapi.InstanceGroupList{})
+	err := c.AdjustNeedUpdate(groups)
 	assert.NoError(t, err, "AddAnnotatedNodesToGroups")
 
 	assertGroupNeedUpdate(t, groups, "node-1", "node-1a", "node-1b")
@@ -776,7 +776,7 @@ func TestAddAnnotatedNodesToNeedsUpdateNodesMissing(t *testing.T) {
 	groups["node-1"].Ready[0].Node = nil
 	groups["node-1"].NeedUpdate[0].Node = nil
 
-	err := c.AdjustNeedUpdate(groups, &kopsapi.InstanceGroupList{})
+	err := c.AdjustNeedUpdate(groups)
 	assert.NoError(t, err, "AddAnnotatedNodesToGroups")
 }
 

upup/models/bindata.go

@@ -3889,9 +3889,7 @@ spec:
           secretName: cilium-ipsec-keys
 {{ end }}
   updateStrategy:
-    rollingUpdate:
+    type: OnDelete
-      maxUnavailable: 2
-    type: RollingUpdate
 ---
 apiVersion: apps/v1
 kind: Deployment

upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.12-v1.8.yaml.template

@@ -685,9 +685,7 @@ spec:
           secretName: cilium-ipsec-keys
 {{ end }}
   updateStrategy:
-    rollingUpdate:
+    type: OnDelete
-      maxUnavailable: 2
-    type: RollingUpdate
 ---
 apiVersion: apps/v1
 kind: Deployment

upup/pkg/fi/cloudup/bootstrapchannelbuilder.go

@@ -877,11 +877,12 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann
 				location := key + "/" + id + "-v1.8.yaml"
 
 				addons.Spec.Addons = append(addons.Spec.Addons, &channelsapi.AddonSpec{
-					Name:     fi.String(key),
+					Name:               fi.String(key),
-					Version:  fi.String(version),
+					Version:            fi.String(version),
-					Selector: networkingSelector,
+					Selector:           networkingSelector,
-					Manifest: fi.String(location),
+					Manifest:           fi.String(location),
-					Id:       id,
+					Id:                 id,
+					NeedsRollingUpdate: "all",
 				})
 			}
 		}

upup/pkg/fi/cloudup/tests/bootstrapchannelbuilder/cilium/manifest.yaml

@@ -70,8 +70,9 @@ spec:
     version: 1.15.0
   - id: k8s-1.12
     manifest: networking.cilium.io/k8s-1.12-v1.8.yaml
-    manifestHash: d3ec6a179bbf7de2fdc6d34190cc31819ece539b
+    manifestHash: 67853b52a456f1b701ada61ecab28c8c1f615183
     name: networking.cilium.io
+    needsRollingUpdate: all
     selector:
       role.kubernetes.io/networking: "1"
     version: 1.8.0-kops.1