Simple generation check to see if instance groups may need updated

Ignoring replace with no spec changes Updating replace cancellation to only not set generation, instead of not performing the update Bazel updates Setting generation in common clientset code
kubernetes · May 28, 2019 · c723e44 · c723e44
1 parent ee58507
commit c723e44
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 5 deletions.
diff --git a/pkg/client/simple/vfsclientset/cluster.go b/pkg/client/simple/vfsclientset/cluster.go
@@ -22,6 +22,7 @@ import (
 	"strings"
 	"time"
 
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -140,6 +141,10 @@ func (r *ClusterVFS) Update(c *api.Cluster, status *api.ClusterStatus) (*api.Clu
 		return nil, err
 	}
 
+	if !apiequality.Semantic.DeepEqual(old.Spec, c.Spec) {
+		c.SetGeneration(old.GetGeneration() + 1)
+	}
+
 	if err := r.writeConfig(c, r.basePath.Join(clusterName, registry.PathCluster), c, vfs.WriteOptionOnlyIfExists); err != nil {
 		if os.IsNotExist(err) {
 			return nil, err

diff --git a/pkg/client/simple/vfsclientset/instancegroup.go b/pkg/client/simple/vfsclientset/instancegroup.go
@@ -19,6 +19,7 @@ package vfsclientset
 import (
 	"fmt"
 
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -138,7 +139,17 @@ func (c *InstanceGroupVFS) Create(g *api.InstanceGroup) (*api.InstanceGroup, err
 }
 
 func (c *InstanceGroupVFS) Update(g *api.InstanceGroup) (*api.InstanceGroup, error) {
-	err := c.update(c.cluster, g)
+
+	old, err := c.Get(g.Name, metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	if !apiequality.Semantic.DeepEqual(old.Spec, g.Spec) {
+		g.SetGeneration(old.GetGeneration() + 1)
+	}
+
+	err = c.update(c.cluster, g)
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/model/openstackmodel/servergroup.go b/pkg/model/openstackmodel/servergroup.go
@@ -60,6 +60,8 @@ func (b *ServerGroupModelBuilder) buildInstances(c *fi.ModelBuilderContext, sg *
 	}
 	igMeta["k8s"] = b.ClusterName()
 	igMeta["KopsInstanceGroup"] = ig.Name
+	igMeta[openstack.INSTANCE_GROUP_GENERATION] = fmt.Sprintf("%d", ig.GetGeneration())
+	igMeta[openstack.CLUSTER_GENERATION] = fmt.Sprintf("%d", b.Cluster.GetGeneration())
 
 	startupScript, err := b.BootstrapScript.ResourceNodeUp(ig, b.Cluster)
 	if err != nil {

diff --git a/upup/pkg/fi/cloudup/openstack/cloud.go b/upup/pkg/fi/cloudup/openstack/cloud.go
@@ -528,7 +528,7 @@ func (c *openstackCloud) GetCloudGroups(cluster *kops.Cluster, instancegroups []
 			}
 			continue
 		}
-		groups[instancegroup.ObjectMeta.Name], err = c.osBuildCloudInstanceGroup(instancegroup, &grp, nodeMap)
+		groups[instancegroup.ObjectMeta.Name], err = c.osBuildCloudInstanceGroup(cluster, instancegroup, &grp, nodeMap)
 		if err != nil {
 			return nil, fmt.Errorf("error getting cloud instance group %q: %v", instancegroup.ObjectMeta.Name, err)
 		}

diff --git a/upup/pkg/fi/cloudup/openstack/instance.go b/upup/pkg/fi/cloudup/openstack/instance.go
@@ -26,6 +26,11 @@ import (
 	"k8s.io/kops/util/pkg/vfs"
 )
 
+const (
+	INSTANCE_GROUP_GENERATION = "ig_generation"
+	CLUSTER_GENERATION        = "cluster_generation"
+)
+
 func (c *openstackCloud) CreateInstance(opt servers.CreateOptsBuilder) (*servers.Server, error) {
 	var server *servers.Server
 

diff --git a/upup/pkg/fi/cloudup/openstack/server_group.go b/upup/pkg/fi/cloudup/openstack/server_group.go
@@ -18,6 +18,7 @@ package openstack
 
 import (
 	"fmt"
+	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
 
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/servergroups"
 	v1 "k8s.io/api/core/v1"
@@ -99,7 +100,7 @@ func matchInstanceGroup(name string, clusterName string, instancegroups []*kops.
 	return instancegroup, nil
 }
 
-func (c *openstackCloud) osBuildCloudInstanceGroup(ig *kops.InstanceGroup, g *servergroups.ServerGroup, nodeMap map[string]*v1.Node) (*cloudinstances.CloudInstanceGroup, error) {
+func (c *openstackCloud) osBuildCloudInstanceGroup(cluster *kops.Cluster, ig *kops.InstanceGroup, g *servergroups.ServerGroup, nodeMap map[string]*v1.Node) (*cloudinstances.CloudInstanceGroup, error) {
 	newLaunchConfigName := g.Name
 	cg := &cloudinstances.CloudInstanceGroup{
 		HumanName:     newLaunchConfigName,
@@ -114,8 +115,16 @@ func (c *openstackCloud) osBuildCloudInstanceGroup(ig *kops.InstanceGroup, g *se
 			klog.Warningf("ignoring instance with no instance id: %s", i)
 			continue
 		}
-		// TODO: how we should implement this, OS does not have launchconfigs? Should we somehow use tags in servergroups and in instances
-		err := cg.NewCloudInstanceGroupMember(instanceId, newLaunchConfigName, newLaunchConfigName+"-updatealways", nodeMap)
+		server, err := servers.Get(c.ComputeClient(), instanceId).Extract()
+		if err != nil {
+			return nil, fmt.Errorf("Failed to get instance group member: %v", err)
+		}
+		igObservedGeneration := server.Metadata[INSTANCE_GROUP_GENERATION]
+		clusterObservedGeneration := server.Metadata[CLUSTER_GENERATION]
+		observedName := fmt.Sprintf("%s-%s", clusterObservedGeneration, igObservedGeneration)
+		generationName := fmt.Sprintf("%d-%d", cluster.GetGeneration(), ig.Generation)
+
+		err = cg.NewCloudInstanceGroupMember(instanceId, generationName, observedName, nodeMap)
 		if err != nil {
 			return nil, fmt.Errorf("error creating cloud instance group member: %v", err)
 		}