From 890365c18952062ede67a2192c11459aeb55e10b Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Mon, 17 Feb 2020 16:48:53 +0200 Subject: [PATCH 1/3] ci: List PRs in release notes --- .circleci/config.yml | 4 +--- test/goreleaser.sh | 4 ---- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 7375d3d3..c9093f34 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -35,9 +35,6 @@ jobs: - run: name: Verify code gen command: make test-codegen - - run: - name: Release notes - command: make release-notes - save_cache: key: go-mod-v3-{{ checksum "go.sum" }} paths: @@ -71,6 +68,7 @@ jobs: - restore_cache: keys: - go-mod-v3-{{ checksum "go.sum" }} + - run: make release-notes - run: test/goreleaser.sh e2e-istio-testing: diff --git a/test/goreleaser.sh b/test/goreleaser.sh index 88d9151a..f8155be9 100755 --- a/test/goreleaser.sh +++ b/test/goreleaser.sh @@ -3,8 +3,6 @@ set -e TAR_FILE="/tmp/goreleaser.tar.gz" RELEASES_URL="https://github.com/goreleaser/goreleaser/releases" -GH_REL_DIR="github-release-notes-linux-amd64-0.2.0" -GH_REL_URL="https://github.com/buchanae/github-release-notes/releases/download/0.2.0/${GH_REL_DIR}.tar.gz" test -z "$TMPDIR" && TMPDIR="$(mktemp -d)" last_version() { @@ -23,8 +21,6 @@ download() { rm -f "$TAR_FILE" curl -s -L -o "$TAR_FILE" \ "$RELEASES_URL/download/$VERSION/goreleaser_$(uname -s)_$(uname -m).tar.gz" - - curl -sSL ${GH_REL_URL} | tar xz && sudo mv ${GH_REL_DIR}/github-release-notes /usr/local/bin/ && rm -rf ${GH_REL_DIR} } download From f5182061efd597894a35cedfba3bb1baadd72db5 Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Mon, 17 Feb 2020 20:00:35 +0200 Subject: [PATCH 2/3] Compute spec hash with spew instead of hashstructure --- go.mod | 2 +- go.sum | 2 - pkg/canary/deployment_controller_test.go | 79 +++++++++++++++++------- pkg/canary/fixture.go | 6 ++ pkg/canary/spec.go | 33 +++++++--- pkg/canary/status.go | 10 +-- 6 files changed, 91 insertions(+), 41 deletions(-) diff --git a/go.mod b/go.mod index f353020b..b06cf195 100644 --- a/go.mod +++ b/go.mod @@ -4,11 +4,11 @@ go 1.13 require ( github.com/Masterminds/semver/v3 v3.0.3 + github.com/davecgh/go-spew v1.1.1 github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef // indirect github.com/google/go-cmp v0.3.0 github.com/googleapis/gnostic v0.2.0 // indirect github.com/imdario/mergo v0.3.7 // indirect - github.com/mitchellh/hashstructure v1.0.0 github.com/pkg/errors v0.8.1 github.com/prometheus/client_golang v1.0.0 go.uber.org/atomic v1.3.2 // indirect diff --git a/go.sum b/go.sum index b46cce59..61d1edd0 100644 --- a/go.sum +++ b/go.sum @@ -119,8 +119,6 @@ github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN github.com/mailru/easyjson v0.7.0/go.mod h1:KAzv3t3aY1NaHWoQz1+4F1ccyAH66Jk7yos7ldAVICs= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= -github.com/mitchellh/hashstructure v1.0.0 h1:ZkRJX1CyOoTkar7p/mLS5TZU4nJ1Rn/F8u9dGS02Q3Y= -github.com/mitchellh/hashstructure v1.0.0/go.mod h1:QjSHrPWS+BGUVBYkbTZWEnOh3G1DutKwClXU/ABz6AQ= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= diff --git a/pkg/canary/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go index 0ea6f5e7..884adaf8 100644 --- a/pkg/canary/deployment_controller_test.go +++ b/pkg/canary/deployment_controller_test.go @@ -1,9 +1,11 @@ package canary import ( - "k8s.io/apimachinery/pkg/api/errors" "testing" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" @@ -39,29 +41,6 @@ func TestCanaryDeployer_Sync(t *testing.T) { } } -func TestCanaryDeployer_IsNewSpec(t *testing.T) { - mocks := newFixture() - err := mocks.deployer.Initialize(mocks.canary, true) - if err != nil { - t.Fatal(err.Error()) - } - - dep2 := newTestDeploymentV2() - _, err = mocks.kubeClient.AppsV1().Deployments("default").Update(dep2) - if err != nil { - t.Fatal(err.Error()) - } - - isNew, err := mocks.deployer.HasTargetChanged(mocks.canary) - if err != nil { - t.Fatal(err.Error()) - } - - if !isNew { - t.Errorf("Got %v wanted %v", isNew, true) - } -} - func TestCanaryDeployer_Promote(t *testing.T) { mocks := newFixture() err := mocks.deployer.Initialize(mocks.canary, true) @@ -272,3 +251,55 @@ func TestCanaryDeployer_NoConfigTracking(t *testing.T) { t.Errorf("Got config name %v wanted %v", configName, "podinfo-config-vol") } } + +func TestCanaryDeployer_HasTargetChanged(t *testing.T) { + mocks := newFixture() + err := mocks.deployer.Initialize(mocks.canary, true) + if err != nil { + t.Fatal(err.Error()) + } + + canary, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + + // save last applied spec hash + err = mocks.deployer.SyncStatus(canary, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryPhaseInitialized}) + if err != nil { + t.Fatal(err.Error()) + } + + dep, err := mocks.kubeClient.AppsV1().Deployments("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + + depClone := dep.DeepCopy() + depClone.Spec.Template.Spec.Containers[0].Resources = corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: *resource.NewQuantity(100, resource.DecimalExponent), + }, + } + + // update pod spec + _, err = mocks.kubeClient.AppsV1().Deployments("default").Update(depClone) + if err != nil { + t.Fatal(err.Error()) + } + + canary, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + + // detect change + isNew, err := mocks.deployer.HasTargetChanged(canary) + if err != nil { + t.Fatal(err.Error()) + } + + if !isNew { + t.Errorf("Got %v wanted %v", isNew, true) + } +} diff --git a/pkg/canary/fixture.go b/pkg/canary/fixture.go index 4a17ccfc..371dddd5 100644 --- a/pkg/canary/fixture.go +++ b/pkg/canary/fixture.go @@ -6,6 +6,7 @@ import ( appsv1 "k8s.io/api/apps/v1" hpav2 "k8s.io/api/autoscaling/v2beta1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" @@ -244,6 +245,11 @@ func newTestDeployment() *appsv1.Deployment { "./podinfo", "--port=9898", }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: *resource.NewQuantity(1000, resource.DecimalExponent), + }, + }, Args: nil, WorkingDir: "", Ports: []corev1.ContainerPort{ diff --git a/pkg/canary/spec.go b/pkg/canary/spec.go index 4da124f3..99467cca 100644 --- a/pkg/canary/spec.go +++ b/pkg/canary/spec.go @@ -2,29 +2,48 @@ package canary import ( "fmt" + "hash/fnv" + + "github.com/davecgh/go-spew/spew" + "k8s.io/apimachinery/pkg/util/rand" - "github.com/mitchellh/hashstructure" flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" ) +// hasSpecChanged computes the hash of the spec and compares it with the +// last applied spec, if the last applied hash is different but not equal +// to last promoted one the it returns true func hasSpecChanged(cd *flaggerv1.Canary, spec interface{}) (bool, error) { if cd.Status.LastAppliedSpec == "" { return true, nil } - newHash, err := hashstructure.Hash(spec, nil) - if err != nil { - return false, fmt.Errorf("hash error %v", err) - } + newHash := computeHash(spec) // do not trigger a canary deployment on manual rollback - if cd.Status.LastPromotedSpec == fmt.Sprintf("%d", newHash) { + if cd.Status.LastPromotedSpec == newHash { return false, nil } - if cd.Status.LastAppliedSpec != fmt.Sprintf("%d", newHash) { + if cd.Status.LastAppliedSpec != newHash { return true, nil } return false, nil } + +// computeHash returns a hash value calculated from a spec using the spew library +// which follows pointers and prints actual values of the nested objects +// ensuring the hash does not change when a pointer changes. +func computeHash(spec interface{}) string { + hasher := fnv.New32a() + printer := spew.ConfigState{ + Indent: " ", + SortKeys: true, + DisableMethods: true, + SpewKeys: true, + } + printer.Fprintf(hasher, "%#v", spec) + + return rand.SafeEncodeString(fmt.Sprint(hasher.Sum32())) +} diff --git a/pkg/canary/status.go b/pkg/canary/status.go index 3872e1ae..4f2f655f 100644 --- a/pkg/canary/status.go +++ b/pkg/canary/status.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "github.com/mitchellh/hashstructure" ex "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -36,13 +35,10 @@ func (c *DeploymentController) SyncStatus(cd *flaggerv1.Canary, status flaggerv1 } func syncCanaryStatus(flaggerClient clientset.Interface, cd *flaggerv1.Canary, status flaggerv1.CanaryStatus, canaryResource interface{}, setAll func(cdCopy *flaggerv1.Canary)) error { - hash, err := hashstructure.Hash(canaryResource, nil) - if err != nil { - return ex.Wrap(err, "SyncStatus hash error") - } + hash := computeHash(canaryResource) firstTry := true - err = retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) { + err := retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) { var selErr error if !firstTry { cd, selErr = flaggerClient.FlaggerV1beta1().Canaries(cd.Namespace).Get(cd.GetName(), metav1.GetOptions{}) @@ -56,7 +52,7 @@ func syncCanaryStatus(flaggerClient clientset.Interface, cd *flaggerv1.Canary, s cdCopy.Status.CanaryWeight = status.CanaryWeight cdCopy.Status.FailedChecks = status.FailedChecks cdCopy.Status.Iterations = status.Iterations - cdCopy.Status.LastAppliedSpec = fmt.Sprintf("%d", hash) + cdCopy.Status.LastAppliedSpec = fmt.Sprintf("%s", hash) cdCopy.Status.LastTransitionTime = metav1.Now() setAll(cdCopy) From c3b1ee6dae27f957ffe831c2a622442a252a137a Mon Sep 17 00:00:00 2001 From: stefanprodan Date: Mon, 17 Feb 2020 20:45:59 +0200 Subject: [PATCH 3/3] Add test for last promoted hash --- Makefile | 3 +- pkg/canary/deployment_controller_test.go | 79 +++++++++++++++++++++++- pkg/canary/status.go | 2 +- 3 files changed, 78 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 64dc3156..dd9c42fa 100644 --- a/Makefile +++ b/Makefile @@ -108,8 +108,7 @@ release-set: fmt version-set helm-package release-notes: cd /tmp && GH_REL_URL="https://github.com/buchanae/github-release-notes/releases/download/0.2.0/github-release-notes-linux-amd64-0.2.0.tar.gz" && \ - curl -sSL $${GH_REL_URL} | tar xz && sudo mv github-release-notes /usr/local/bin/ && \ - github-release-notes -org weaveworks -repo flagger -since-latest-release + curl -sSL $${GH_REL_URL} | tar xz && sudo mv github-release-notes /usr/local/bin/ reset-test: kubectl delete -f ./artifacts/namespaces diff --git a/pkg/canary/deployment_controller_test.go b/pkg/canary/deployment_controller_test.go index 884adaf8..d7792436 100644 --- a/pkg/canary/deployment_controller_test.go +++ b/pkg/canary/deployment_controller_test.go @@ -259,13 +259,22 @@ func TestCanaryDeployer_HasTargetChanged(t *testing.T) { t.Fatal(err.Error()) } + // save last applied hash canary, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get("podinfo", metav1.GetOptions{}) if err != nil { t.Fatal(err.Error()) } + err = mocks.deployer.SyncStatus(canary, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryPhaseInitializing}) + if err != nil { + t.Fatal(err.Error()) + } - // save last applied spec hash - err = mocks.deployer.SyncStatus(canary, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryPhaseInitialized}) + // save last promoted hash + canary, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + err = mocks.deployer.SetStatusPhase(canary, flaggerv1.CanaryPhaseInitialized) if err != nil { t.Fatal(err.Error()) } @@ -293,12 +302,76 @@ func TestCanaryDeployer_HasTargetChanged(t *testing.T) { t.Fatal(err.Error()) } - // detect change + // detect change in last applied spec isNew, err := mocks.deployer.HasTargetChanged(canary) if err != nil { t.Fatal(err.Error()) } + if !isNew { + t.Errorf("Got %v wanted %v", isNew, true) + } + // save hash + err = mocks.deployer.SyncStatus(canary, flaggerv1.CanaryStatus{Phase: flaggerv1.CanaryPhaseProgressing}) + if err != nil { + t.Fatal(err.Error()) + } + + dep, err = mocks.kubeClient.AppsV1().Deployments("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + + depClone = dep.DeepCopy() + depClone.Spec.Template.Spec.Containers[0].Resources = corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: *resource.NewQuantity(1000, resource.DecimalExponent), + }, + } + + // update pod spec + _, err = mocks.kubeClient.AppsV1().Deployments("default").Update(depClone) + if err != nil { + t.Fatal(err.Error()) + } + + canary, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + + // ignore change as hash should be the same with last promoted + isNew, err = mocks.deployer.HasTargetChanged(canary) + if err != nil { + t.Fatal(err.Error()) + } + if isNew { + t.Errorf("Got %v wanted %v", isNew, false) + } + + depClone = dep.DeepCopy() + depClone.Spec.Template.Spec.Containers[0].Resources = corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: *resource.NewQuantity(600, resource.DecimalExponent), + }, + } + + // update pod spec + _, err = mocks.kubeClient.AppsV1().Deployments("default").Update(depClone) + if err != nil { + t.Fatal(err.Error()) + } + + canary, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get("podinfo", metav1.GetOptions{}) + if err != nil { + t.Fatal(err.Error()) + } + + // detect change + isNew, err = mocks.deployer.HasTargetChanged(canary) + if err != nil { + t.Fatal(err.Error()) + } if !isNew { t.Errorf("Got %v wanted %v", isNew, true) } diff --git a/pkg/canary/status.go b/pkg/canary/status.go index 4f2f655f..ef10edc1 100644 --- a/pkg/canary/status.go +++ b/pkg/canary/status.go @@ -52,7 +52,7 @@ func syncCanaryStatus(flaggerClient clientset.Interface, cd *flaggerv1.Canary, s cdCopy.Status.CanaryWeight = status.CanaryWeight cdCopy.Status.FailedChecks = status.FailedChecks cdCopy.Status.Iterations = status.Iterations - cdCopy.Status.LastAppliedSpec = fmt.Sprintf("%s", hash) + cdCopy.Status.LastAppliedSpec = hash cdCopy.Status.LastTransitionTime = metav1.Now() setAll(cdCopy)