From 9c038f30d665dcbc2c7c870385afa4ca73529a17 Mon Sep 17 00:00:00 2001 From: James Ravn Date: Tue, 29 Jan 2019 20:04:17 +0000 Subject: [PATCH] fix(tiller): respect resource policy on upgrade (#5225) Don't delete a resource on upgrade if it is annotated with helm.io/resource-policy=keep. This can cause data loss for users if the annotation is ignored (e.g. for a PVC). Closes #3673 Signed-off-by: James Ravn Signed-off-by: Kevin Labesse --- pkg/kube/client.go | 18 +++++++++++++++++- pkg/kube/client_test.go | 15 +++++++++++++++ pkg/kube/resource_policy.go | 26 ++++++++++++++++++++++++++ pkg/tiller/resource_policy.go | 13 ++----------- 4 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 pkg/kube/resource_policy.go diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 4a387d524..e897aced6 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -23,11 +23,12 @@ import ( goerrors "errors" "fmt" "io" + "k8s.io/apimachinery/pkg/api/meta" "log" "strings" "time" - jsonpatch "github.com/evanphx/json-patch" + "github.com/evanphx/json-patch" appsv1 "k8s.io/api/apps/v1" appsv1beta1 "k8s.io/api/apps/v1beta1" appsv1beta2 "k8s.io/api/apps/v1beta2" @@ -60,6 +61,8 @@ const MissingGetHeader = "==> MISSING\nKIND\t\tNAME\n" // ErrNoObjectsVisited indicates that during a visit operation, no matching objects were found. var ErrNoObjectsVisited = goerrors.New("no objects visited") +var metadataAccessor = meta.NewAccessor() + // Client represents a client capable of communicating with the Kubernetes API. type Client struct { cmdutil.Factory @@ -308,6 +311,19 @@ func (c *Client) Update(namespace string, originalReader, targetReader io.Reader for _, info := range original.Difference(target) { c.Log("Deleting %q in %s...", info.Name, info.Namespace) + + if err := info.Get(); err != nil { + c.Log("Unable to get obj %q, err: %s", info.Name, err) + } + annotations, err := metadataAccessor.Annotations(info.Object) + if err != nil { + c.Log("Unable to get annotations on %q, err: %s", info.Name, err) + } + if annotations != nil && annotations[ResourcePolicyAnno] == KeepPolicy { + c.Log("Skipping delete of %q due to annotation [%s=%s]", info.Name, ResourcePolicyAnno, KeepPolicy) + continue + } + if err := deleteResource(info); err != nil { c.Log("Failed to delete %q, err: %s", info.Name, err) } diff --git a/pkg/kube/client_test.go b/pkg/kube/client_test.go index de33881c8..aa21b937c 100644 --- a/pkg/kube/client_test.go +++ b/pkg/kube/client_test.go @@ -151,6 +151,8 @@ func TestUpdate(t *testing.T) { return newResponse(200, &listB.Items[1]) case p == "/namespaces/default/pods/squid" && m == "DELETE": return newResponse(200, &listB.Items[1]) + case p == "/namespaces/default/pods/squid" && m == "GET": + return newResponse(200, &listA.Items[2]) default: t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) return nil, nil @@ -183,6 +185,7 @@ func TestUpdate(t *testing.T) { "/namespaces/default/pods/otter:GET", "/namespaces/default/pods/dolphin:GET", "/namespaces/default/pods:POST", + "/namespaces/default/pods/squid:GET", "/namespaces/default/pods/squid:DELETE", } if len(expectedActions) != len(actions) { @@ -194,6 +197,18 @@ func TestUpdate(t *testing.T) { t.Errorf("expected %s request got %s", v, actions[k]) } } + + // Test resource policy is respected + actions = nil + listA.Items[2].ObjectMeta.Annotations = map[string]string{ResourcePolicyAnno: KeepPolicy} + if err := c.Update(v1.NamespaceDefault, objBody(&listA), objBody(&listB), false, false, 0, false); err != nil { + t.Fatal(err) + } + for _, v := range actions { + if v == "/namespaces/default/pods/squid:DELETE" { + t.Errorf("should not have deleted squid - it has helm.sh/resource-policy=keep") + } + } } func TestBuild(t *testing.T) { diff --git a/pkg/kube/resource_policy.go b/pkg/kube/resource_policy.go new file mode 100644 index 000000000..45cebcba8 --- /dev/null +++ b/pkg/kube/resource_policy.go @@ -0,0 +1,26 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +// ResourcePolicyAnno is the annotation name for a resource policy +const ResourcePolicyAnno = "helm.sh/resource-policy" + +// KeepPolicy is the resource policy type for keep +// +// This resource policy type allows resources to skip being deleted +// during an uninstallRelease action. +const KeepPolicy = "keep" diff --git a/pkg/tiller/resource_policy.go b/pkg/tiller/resource_policy.go index cca2391d8..aa9c5d2bd 100644 --- a/pkg/tiller/resource_policy.go +++ b/pkg/tiller/resource_policy.go @@ -24,15 +24,6 @@ import ( "k8s.io/helm/pkg/tiller/environment" ) -// resourcePolicyAnno is the annotation name for a resource policy -const resourcePolicyAnno = "helm.sh/resource-policy" - -// keepPolicy is the resource policy type for keep -// -// This resource policy type allows resources to skip being deleted -// during an uninstallRelease action. -const keepPolicy = "keep" - func filterManifestsToKeep(manifests []Manifest) ([]Manifest, []Manifest) { remaining := []Manifest{} keep := []Manifest{} @@ -43,14 +34,14 @@ func filterManifestsToKeep(manifests []Manifest) ([]Manifest, []Manifest) { continue } - resourcePolicyType, ok := m.Head.Metadata.Annotations[resourcePolicyAnno] + resourcePolicyType, ok := m.Head.Metadata.Annotations[kube.ResourcePolicyAnno] if !ok { remaining = append(remaining, m) continue } resourcePolicyType = strings.ToLower(strings.TrimSpace(resourcePolicyType)) - if resourcePolicyType == keepPolicy { + if resourcePolicyType == kube.KeepPolicy { keep = append(keep, m) }