Skip to content

Commit

Permalink
Merge pull request kubernetes#7133 from granular-ryanbonham/squash_im…
Browse files Browse the repository at this point in the history 
…prove_channel_updates

Improve channel updates
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Jul 22, 2019
2 parents 2cbf735 + cd0075d commit d961f98
Show file tree
Hide file tree
Showing 12 changed files with 244 additions and 142 deletions.
3 changes: 3 additions & 0 deletions channels/pkg/api/channel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ type AddonSpec struct {
// Manifest is the URL to the manifest that should be applied
Manifest *string `json:"manifest,omitempty"`

// Manifesthash is the sha1 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
KubernetesVersion string `json:"kubernetesVersion,omitempty"`

Expand Down
35 changes: 23 additions & 12 deletions channels/pkg/channels/addon.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,12 @@ func (m *AddonMenu) MergeAddons(o *AddonMenu) {
}

func (a *Addon) ChannelVersion() *ChannelVersion {

return &ChannelVersion{
Channel: &a.ChannelName,
Version: a.Spec.Version,
Id: a.Spec.Id,
Channel: &a.ChannelName,
Version: a.Spec.Version,
Id: a.Spec.Id,
ManifestHash: a.Spec.ManifestHash,
}
}

Expand All @@ -85,6 +87,7 @@ func (a *Addon) buildChannel() *Channel {
}
return channel
}

func (a *Addon) GetRequiredUpdates(k8sClient kubernetes.Interface) (*AddonUpdate, error) {
newVersion := a.ChannelVersion()

Expand All @@ -106,14 +109,7 @@ func (a *Addon) GetRequiredUpdates(k8sClient kubernetes.Interface) (*AddonUpdate
}, nil
}

func (a *Addon) EnsureUpdated(k8sClient kubernetes.Interface) (*AddonUpdate, error) {
required, err := a.GetRequiredUpdates(k8sClient)
if err != nil {
return nil, err
}
if required == nil {
return nil, nil
}
func (a *Addon) GetManifestFullUrl() (*url.URL, error) {

if a.Spec.Manifest == nil || *a.Spec.Manifest == "" {
return nil, field.Required(field.NewPath("Spec", "Manifest"), "")
Expand All @@ -127,11 +123,26 @@ func (a *Addon) EnsureUpdated(k8sClient kubernetes.Interface) (*AddonUpdate, err
if !manifestURL.IsAbs() {
manifestURL = a.ChannelLocation.ResolveReference(manifestURL)
}
return manifestURL, nil
}

func (a *Addon) EnsureUpdated(k8sClient kubernetes.Interface) (*AddonUpdate, error) {
required, err := a.GetRequiredUpdates(k8sClient)
if err != nil {
return nil, err
}
if required == nil {
return nil, nil
}
manifestURL, err := a.GetManifestFullUrl()
if err != nil {
return nil, err
}
klog.Infof("Applying update from %q", manifestURL)

err = Apply(manifestURL.String())
if err != nil {
return nil, fmt.Errorf("error applying update from %q: %v", manifest, err)
return nil, fmt.Errorf("error applying update from %q: %v", manifestURL, err)
}

channel := a.buildChannel()
Expand Down
60 changes: 38 additions & 22 deletions channels/pkg/channels/addons_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,60 +85,76 @@ func Test_Replacement(t *testing.T) {
}{
// With no id, update if and only if newer semver
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: ""},
New: &ChannelVersion{Version: s("1.0.0"), Id: ""},
Old: &ChannelVersion{Version: s("1.0.0"), Id: "", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.0.0"), Id: "", ManifestHash: ""},
Replaces: false,
},
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: ""},
New: &ChannelVersion{Version: s("1.0.1"), Id: ""},
Old: &ChannelVersion{Version: s("1.0.0"), Id: "", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.0.1"), Id: "", ManifestHash: ""},
Replaces: true,
},
{
Old: &ChannelVersion{Version: s("1.0.1"), Id: ""},
New: &ChannelVersion{Version: s("1.0.0"), Id: ""},
Old: &ChannelVersion{Version: s("1.0.1"), Id: "", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.0.0"), Id: "", ManifestHash: ""},
Replaces: false,
},
{
Old: &ChannelVersion{Version: s("1.1.0"), Id: ""},
New: &ChannelVersion{Version: s("1.1.1"), Id: ""},
Old: &ChannelVersion{Version: s("1.1.0"), Id: "", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.1.1"), Id: "", ManifestHash: ""},
Replaces: true,
},
{
Old: &ChannelVersion{Version: s("1.1.1"), Id: ""},
New: &ChannelVersion{Version: s("1.1.0"), Id: ""},
Old: &ChannelVersion{Version: s("1.1.1"), Id: "", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.1.0"), Id: "", ManifestHash: ""},
Replaces: false,
},

// With id, update if different id and same version, otherwise follow semver
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a"},
New: &ChannelVersion{Version: s("1.0.0"), Id: "a"},
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: ""},
Replaces: false,
},
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a"},
New: &ChannelVersion{Version: s("1.0.0"), Id: "b"},
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.0.0"), Id: "b", ManifestHash: ""},
Replaces: true,
},
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: "b"},
New: &ChannelVersion{Version: s("1.0.0"), Id: "a"},
Old: &ChannelVersion{Version: s("1.0.0"), Id: "b", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: ""},
Replaces: true,
},
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a"},
New: &ChannelVersion{Version: s("1.0.1"), Id: "a"},
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.0.1"), Id: "a", ManifestHash: ""},
Replaces: true,
},
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a"},
New: &ChannelVersion{Version: s("1.0.1"), Id: "a"},
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.0.1"), Id: "a", ManifestHash: ""},
Replaces: true,
},
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a"},
New: &ChannelVersion{Version: s("1.0.1"), Id: "a"},
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.0.1"), Id: "a", ManifestHash: ""},
Replaces: true,
},
//Test ManifestHash Changes
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: "3544de6578b2b582c0323b15b7b05a28c60b9430"},
New: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: "3544de6578b2b582c0323b15b7b05a28c60b9430"},
Replaces: false,
},
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: ""},
New: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: "3544de6578b2b582c0323b15b7b05a28c60b9430"},
Replaces: true,
},
{
Old: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: "3544de6578b2b582c0323b15b7b05a28c60b9430"},
New: &ChannelVersion{Version: s("1.0.0"), Id: "a", ManifestHash: "ea9e79bf29adda450446487d65a8fc6b3fdf8c2b"},
Replaces: true,
},
}
Expand Down
35 changes: 26 additions & 9 deletions channels/pkg/channels/channel_version.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ type Channel struct {
}

type ChannelVersion struct {
Version *string `json:"version,omitempty"`
Channel *string `json:"channel,omitempty"`
Id string `json:"id,omitempty"`
Version *string `json:"version,omitempty"`
Channel *string `json:"channel,omitempty"`
Id string `json:"id,omitempty"`
ManifestHash string `json:"manifestHash,omitempty"`
}

func stringValue(s *string) string {
Expand All @@ -54,6 +55,9 @@ func (c *ChannelVersion) String() string {
if c.Id != "" {
s += " Id=" + c.Id
}
if c.ManifestHash != "" {
s += " ManifestHash=" + c.ManifestHash
}
return s
}

Expand Down Expand Up @@ -98,8 +102,10 @@ func (c *Channel) AnnotationName() string {
}

func (c *ChannelVersion) replaces(existing *ChannelVersion) bool {
klog.V(4).Infof("Checking existing channel: %v compared to new channel: %v", existing, c)
if existing.Version != nil {
if c.Version == nil {
klog.V(4).Infof("New Version info missing")
return false
}
cVersion, err := semver.ParseTolerant(*c.Version)
Expand All @@ -113,20 +119,31 @@ func (c *ChannelVersion) replaces(existing *ChannelVersion) bool {
return true
}
if cVersion.LT(existingVersion) {
klog.V(4).Infof("New Version is less then old")
return false
} else if cVersion.GT(existingVersion) {
klog.V(4).Infof("New Version is greater then old")
return true
} else {
// Same version; check ids
if c.Id == existing.Id {
return false
// Same id; check manifests
if c.ManifestHash == existing.ManifestHash {
klog.V(4).Infof("Manifest Match")
return false
} else {
klog.V(4).Infof("Channels had same version and ids %q, %q but different ManifestHash (%q vs %q); will replace", *c.Version, c.Id, c.ManifestHash, existing.ManifestHash)
}
} else {
klog.V(4).Infof("Channels had same version %q but different ids (%q vs %q); will replace", *c.Version, c.Id, existing.Id)
}
klog.V(4).Infof("Channels had same version %q but different ids (%q vs %q); will replace", *c.Version, c.Id, existing.Id)
}
} else {
klog.Warningf("Existing ChannelVersion did not have a version; can't perform real version check")
}

klog.Warningf("ChannelVersion did not have a version; can't perform real version check")
if c.Version == nil {
klog.Warningf("New ChannelVersion did not have a version; can't perform real version check")
return false
}

Expand Down Expand Up @@ -167,14 +184,14 @@ func (c *Channel) SetInstalledVersion(k8sClient kubernetes.Interface, version *C
}

annotationPatch := &annotationPatch{Metadata: annotationPatchMetadata{Annotations: map[string]string{c.AnnotationName(): value}}}
annotationPatchJson, err := json.Marshal(annotationPatch)
annotationPatchJSON, err := json.Marshal(annotationPatch)
if err != nil {
return fmt.Errorf("error building annotation patch: %v", err)
}

klog.V(2).Infof("sending patch: %q", string(annotationPatchJson))
klog.V(2).Infof("sending patch: %q", string(annotationPatchJSON))

_, err = k8sClient.CoreV1().Namespaces().Patch(c.Namespace, types.StrategicMergePatchType, annotationPatchJson)
_, err = k8sClient.CoreV1().Namespaces().Patch(c.Namespace, types.StrategicMergePatchType, annotationPatchJSON)
if err != nil {
return fmt.Errorf("error applying annotation to namespace: %v", err)
}
Expand Down
20 changes: 15 additions & 5 deletions docs/addon_manager.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ with a `kubectl apply -f https://...` or with the channels tool.
In future, we may as a convenience make it easy to add optional addons to the kops manifest,
though this will just be a convenience wrapper around doing it manually.

## Update BootStrap Addons

If you want to update the bootstrap addons, you can run the following command to show you which addons need updating. Add `--yes` to actually apply the updates.

**channels apply channel s3://*KOPS_S3_BUCKET*/*CLUSTER_NAME*/addons/bootstrap-channel.yaml**


## Versioning

The channels tool adds a manifest-of-manifests file, of `Kind: Addons`, which allows for a description
Expand Down Expand Up @@ -50,12 +57,15 @@ a few more protocols than `kubectl` - for example `s3://...` for S3 hosted manif

The `version` field gives meaning to the alternative manifests. This is interpreted as a
semver. The channels tool keeps track of the current version installed (currently by means
of an annotation on the `kube-system` namespace), and it will not reapply the same version
of the manifest. This means that a user can edit a deployed addon, and changes will not
be replaced, until a new version of the addon is installed.
of an annotation on the `kube-system` namespace).

The channel tool updates the installed version when any of the following conditions apply.
* The version declared in the addon manifest is greater then the currently installed version.
* The version number's match, but the ids are different
* The version number and ids match, but the hash of the addon's manifest has changed since it was installed.


The long-term direction here is that addons will mostly be configured through a ConfigMap or Secret object,
and that the addon manager will (TODO) not replace the ConfigMap.
This means that a user can edit a deployed addon, and changes will not be replaced, until a new version of the addon is installed. The long-term direction here is that addons will mostly be configured through a ConfigMap or Secret object, and that the addon manager will (TODO) not replace the ConfigMap.

The `selector` determines the objects which make up the addon. This will be used
to construct a `--prune` argument (TODO), so that objects that existed in the
Expand Down
Loading

0 comments on commit d961f98

Please sign in to comment.