Add SystemGeneration to channel version tracker

This allows us to reapply a manifest when we introduce new
functionality, such as pruning.

Otherwise an old version can apply the manifest, mark the manifest as
applied, and we won't reapply.

Author: justinsb

channels/pkg/api/channel.go

@@ -46,7 +46,7 @@ type AddonSpec struct {
 	// Manifest is the URL to the manifest that should be applied
 	Manifest *string `json:"manifest,omitempty"`
 
-	// Manifesthash is the sha256 hash of our manifest
+	// ManifestHash is the sha256 hash of our manifest
 	ManifestHash string `json:"manifestHash,omitempty"`
 
 	// KubernetesVersion is a semver version range on which this version of the addon can be applied

channels/pkg/channels/addon.go

@@ -73,7 +73,7 @@ func (m *AddonMenu) MergeAddons(o *AddonMenu) {
 		if existing == nil {
 			m.Addons[k] = v
 		} else {
-			if v.ChannelVersion().replaces(existing.ChannelVersion()) {
+			if v.ChannelVersion().replaces(k, existing.ChannelVersion()) {
 				m.Addons[k] = v
 			}
 		}
@@ -82,9 +82,10 @@ func (m *AddonMenu) MergeAddons(o *AddonMenu) {
 
 func (a *Addon) ChannelVersion() *ChannelVersion {
 	return &ChannelVersion{
-		Channel:      &a.ChannelName,
-		Id:           a.Spec.Id,
-		ManifestHash: a.Spec.ManifestHash,
+		Channel:          &a.ChannelName,
+		Id:               a.Spec.Id,
+		ManifestHash:     a.Spec.ManifestHash,
+		SystemGeneration: CurrentSystemGeneration,
 	}
 }
 
@@ -120,7 +121,7 @@ func (a *Addon) GetRequiredUpdates(ctx context.Context, k8sClient kubernetes.Int
 		}
 	}
 
-	if existingVersion != nil && !newVersion.replaces(existingVersion) {
+	if existingVersion != nil && !newVersion.replaces(a.Name, existingVersion) {
 		newVersion = nil
 	}
 

channels/pkg/channels/addons.go

@@ -74,7 +74,7 @@ func (a *Addons) GetCurrent(kubernetesVersion semver.Version) (*AddonMenu, error
 		name := addon.Name
 
 		existing := menu.Addons[name]
-		if existing == nil || addon.ChannelVersion().replaces(existing.ChannelVersion()) {
+		if existing == nil || addon.ChannelVersion().replaces(name, existing.ChannelVersion()) {
 			menu.Addons[name] = addon
 		}
 	}

channels/pkg/channels/addons_test.go

@@ -85,30 +85,48 @@ func Test_Filtering(t *testing.T) {
 }
 
 func Test_Replacement(t *testing.T) {
+	hash1 := "3544de6578b2b582c0323b15b7b05a28c60b9430"
+	hash2 := "ea9e79bf29adda450446487d65a8fc6b3fdf8c2b"
+
 	grid := []struct {
 		Old      *ChannelVersion
 		New      *ChannelVersion
 		Replaces bool
 	}{
 		//Test ManifestHash Changes
 		{
-			Old:      &ChannelVersion{Id: "a", ManifestHash: "3544de6578b2b582c0323b15b7b05a28c60b9430"},
-			New:      &ChannelVersion{Id: "a", ManifestHash: "3544de6578b2b582c0323b15b7b05a28c60b9430"},
+			Old:      &ChannelVersion{Id: "a", ManifestHash: hash1},
+			New:      &ChannelVersion{Id: "a", ManifestHash: hash1},
 			Replaces: false,
 		},
 		{
 			Old:      &ChannelVersion{Id: "a", ManifestHash: ""},
-			New:      &ChannelVersion{Id: "a", ManifestHash: "3544de6578b2b582c0323b15b7b05a28c60b9430"},
+			New:      &ChannelVersion{Id: "a", ManifestHash: hash1},
 			Replaces: true,
 		},
 		{
-			Old:      &ChannelVersion{Id: "a", ManifestHash: "3544de6578b2b582c0323b15b7b05a28c60b9430"},
-			New:      &ChannelVersion{Id: "a", ManifestHash: "ea9e79bf29adda450446487d65a8fc6b3fdf8c2b"},
+			Old:      &ChannelVersion{Id: "a", ManifestHash: hash1},
+			New:      &ChannelVersion{Id: "a", ManifestHash: hash2},
 			Replaces: true,
 		},
+		{
+			Old:      &ChannelVersion{Id: "a", ManifestHash: hash1},
+			New:      &ChannelVersion{Id: "a", ManifestHash: hash1, SystemGeneration: 1},
+			Replaces: true,
+		},
+		{
+			Old:      &ChannelVersion{Id: "a", ManifestHash: hash1, SystemGeneration: 1},
+			New:      &ChannelVersion{Id: "a", ManifestHash: hash1},
+			Replaces: false,
+		},
+		{
+			Old:      &ChannelVersion{Id: "a", ManifestHash: hash1, SystemGeneration: 1},
+			New:      &ChannelVersion{Id: "a", ManifestHash: hash1, SystemGeneration: 1},
+			Replaces: false,
+		},
 	}
 	for _, g := range grid {
-		actual := g.New.replaces(g.Old)
+		actual := g.New.replaces(t.Name(), g.Old)
 		if actual != g.Replaces {
 			t.Errorf("unexpected result from %v -> %v, expect %t.  actual %v", g.Old, g.New, g.Replaces, actual)
 		}
@@ -218,7 +236,7 @@ func Test_NeedsRollingUpdate(t *testing.T) {
 		ctx := context.Background()
 
 		annotations := map[string]string{
-			"addons.k8s.io/test": "{\"manifestHash\":\"originalHash\"}",
+			"addons.k8s.io/test": "{\"manifestHash\":\"originalHash\",\"systemGeneration\": 1}",
 		}
 		if len(g.originalAnnotations) > 0 {
 			annotations = g.originalAnnotations

channels/pkg/channels/channel_version.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strconv"
 	"strings"
 
 	certmanager "github.com/jetstack/cert-manager/pkg/client/clientset/versioned"
@@ -38,10 +39,20 @@ type Channel struct {
 	Name      string
 }
 
+// CurrentSystemGeneration holds our current SystemGeneration value.
+// Version history:
+//   0  Pre-history (and the default value); versions prior to prune.
+//   1  Prune functionality introduced.
+const CurrentSystemGeneration = 1
+
 type ChannelVersion struct {
 	Channel      *string `json:"channel,omitempty"`
 	Id           string  `json:"id,omitempty"`
 	ManifestHash string  `json:"manifestHash,omitempty"`
+
+	// SystemGeneration holds the generation of the channels functionality.
+	// It is used so that we reapply when we introduce new features, such as prune.
+	SystemGeneration int `json:"systemGeneration,omitempty"`
 }
 
 func stringValue(s *string) string {
@@ -59,6 +70,7 @@ func (c *ChannelVersion) String() string {
 	if c.ManifestHash != "" {
 		s += " ManifestHash=" + c.ManifestHash
 	}
+	s += " SystemGeneration=" + strconv.Itoa(c.SystemGeneration)
 	return s
 }
 
@@ -102,21 +114,31 @@ func (c *Channel) AnnotationName() string {
 	return AnnotationPrefix + c.Name
 }
 
-func (c *ChannelVersion) replaces(existing *ChannelVersion) bool {
-	klog.V(4).Infof("Checking existing channel: %v compared to new channel: %v", existing, c)
+func (c *ChannelVersion) replaces(name string, existing *ChannelVersion) bool {
+	klog.V(6).Infof("Checking existing config for %q: %v compared to new channel: %v", name, existing, c)
+
+	if c.Id != existing.Id {
+		klog.V(4).Infof("cluster has different ids for %q (%q vs %q); will replace", name, c.Id, existing.Id)
+		return true
+	}
+
+	if c.ManifestHash != existing.ManifestHash {
+		klog.V(4).Infof("cluster has different ManifestHash for %q (%q vs %q); will replace", name, c.ManifestHash, existing.ManifestHash)
+		return true
+	}
 
-	if c.Id == existing.Id {
-		// Same id; check manifests
-		if c.ManifestHash == existing.ManifestHash {
-			klog.V(4).Infof("Manifest Match")
+	if existing.SystemGeneration != c.SystemGeneration {
+		if existing.SystemGeneration > c.SystemGeneration {
+			klog.V(4).Infof("cluster has newer SystemGeneration for %q (%v vs %v), will not replace", name, existing.SystemGeneration, c.SystemGeneration)
 			return false
+		} else {
+			klog.V(4).Infof("cluster has different SystemGeneration for %q (%v vs %v); will replace", name, existing.SystemGeneration, c.SystemGeneration)
+			return true
 		}
-		klog.V(4).Infof("Channels had same ids %q, %q but different ManifestHash (%q vs %q); will replace", c.Id, c.ManifestHash, existing.ManifestHash)
-	} else {
-		klog.V(4).Infof("Channels had different ids (%q vs %q); will replace", c.Id, existing.Id)
 	}
 
-	return true
+	klog.V(4).Infof("manifest Match for %q: %v", name, existing)
+	return false
 }
 
 func (c *Channel) GetInstalledVersion(ctx context.Context, k8sClient kubernetes.Interface) (*ChannelVersion, error) {

nodeup/pkg/model/update_service.go

@@ -92,7 +92,7 @@ func (b *UpdateServiceBuilder) buildDebianPackage(c *fi.ModelBuilderContext) {
 `
 	} else {
 
-		klog.Infof("Detected OS %s; installing %s package", b.Distribution, debianPackageName)
+		klog.Infof("Detected OS %v; installing %s package", b.Distribution, debianPackageName)
 		c.AddTask(&nodetasks.Package{Name: debianPackageName})
 
 		contents = `APT::Periodic::Update-Package-Lists "1";