diff --git a/provider/helm/unversioned_updates.go b/provider/helm/unversioned_updates.go index 8caa61bc..0feb1659 100644 --- a/provider/helm/unversioned_updates.go +++ b/provider/helm/unversioned_updates.go @@ -52,7 +52,6 @@ func checkUnversionedRelease(repo *types.Repository, namespace, name string, cha // checking for impacted images for _, imageDetails := range keelCfg.Images { - imageRef, err := parseImage(vals, &imageDetails) if err != nil { log.WithFields(log.Fields{ diff --git a/provider/kubernetes/approvals_test.go b/provider/kubernetes/approvals_test.go index 806a2fcb..bc50711b 100644 --- a/provider/kubernetes/approvals_test.go +++ b/provider/kubernetes/approvals_test.go @@ -24,8 +24,8 @@ func TestCheckRequestedApproval(t *testing.T) { }, }, } - deployments := []apps_v1.Deployment{ - apps_v1.Deployment{ + deployments := []*apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -95,8 +95,8 @@ func TestApprovedCheck(t *testing.T) { }, }, } - deployments := []apps_v1.Deployment{ - apps_v1.Deployment{ + deployments := []*apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", diff --git a/provider/kubernetes/common.go b/provider/kubernetes/common.go deleted file mode 100644 index 24defd04..00000000 --- a/provider/kubernetes/common.go +++ /dev/null @@ -1,36 +0,0 @@ -package kubernetes - -import ( - "strings" -) - -func addImageToPull(annotations map[string]string, image string) map[string]string { - existing, ok := annotations[forceUpdateImageAnnotation] - if ok { - // check if it's already there - if shouldPullImage(annotations, image) { - // skipping - return annotations - } - - annotations[forceUpdateImageAnnotation] = existing + "," + image - return annotations - } - annotations[forceUpdateImageAnnotation] = image - return annotations -} - -func shouldPullImage(annotations map[string]string, image string) bool { - imagesStr, ok := annotations[forceUpdateImageAnnotation] - if !ok { - return false - } - - images := strings.Split(imagesStr, ",") - for _, img := range images { - if img == image { - return true - } - } - return false -} diff --git a/provider/kubernetes/common_test.go b/provider/kubernetes/common_test.go deleted file mode 100644 index 8cb79eb6..00000000 --- a/provider/kubernetes/common_test.go +++ /dev/null @@ -1,76 +0,0 @@ -package kubernetes - -import ( - "reflect" - "testing" -) - -func Test_addImageToPull(t *testing.T) { - type args struct { - annotations map[string]string - image string - } - tests := []struct { - name string - args args - want map[string]string - }{ - { - name: "empty", - args: args{annotations: make(map[string]string), image: "whatever"}, - want: map[string]string{forceUpdateImageAnnotation: "whatever"}, - }, - { - name: "not empty", - args: args{annotations: map[string]string{forceUpdateImageAnnotation: "foo"}, image: "bar"}, - want: map[string]string{forceUpdateImageAnnotation: "foo,bar"}, - }, - { - name: "not empty with same image", - args: args{annotations: map[string]string{forceUpdateImageAnnotation: "foo"}, image: "foo"}, - want: map[string]string{forceUpdateImageAnnotation: "foo"}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := addImageToPull(tt.args.annotations, tt.args.image); !reflect.DeepEqual(got, tt.want) { - t.Errorf("addImageToPull() = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_shouldPullImage(t *testing.T) { - type args struct { - annotations map[string]string - image string - } - tests := []struct { - name string - args args - want bool - }{ - { - name: "should pull single image", - args: args{annotations: map[string]string{forceUpdateImageAnnotation: "bar"}, image: "bar"}, - want: true, - }, - { - name: "should pull multiple image", - args: args{annotations: map[string]string{forceUpdateImageAnnotation: "foo,bar,whatever"}, image: "bar"}, - want: true, - }, - { - name: "should not pull multiple image", - args: args{annotations: map[string]string{forceUpdateImageAnnotation: "foo,bar,whatever"}, image: "alpha"}, - want: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := shouldPullImage(tt.args.annotations, tt.args.image); got != tt.want { - t.Errorf("shouldPullImage() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/provider/kubernetes/force_update_test.go b/provider/kubernetes/force_update_test.go index 3d005525..fb83144b 100644 --- a/provider/kubernetes/force_update_test.go +++ b/provider/kubernetes/force_update_test.go @@ -1,134 +1,134 @@ package kubernetes -import ( - "testing" - "time" +// import ( +// "testing" +// "time" - "github.com/keel-hq/keel/internal/k8s" - "github.com/keel-hq/keel/types" - "k8s.io/api/core/v1" - "k8s.io/api/extensions/v1beta1" - meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) +// "github.com/keel-hq/keel/internal/k8s" +// "github.com/keel-hq/keel/types" +// "k8s.io/api/core/v1" +// "k8s.io/api/extensions/v1beta1" +// meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +// ) -func TestForceUpdate(t *testing.T) { +// func TestForceUpdate(t *testing.T) { - fp := &fakeImplementer{} +// fp := &fakeImplementer{} - dep := &v1beta1.Deployment{ - meta_v1.TypeMeta{}, - meta_v1.ObjectMeta{ - Name: "deployment-1", - Namespace: "xx", - Labels: map[string]string{types.KeelPolicyLabel: "all"}, - }, - v1beta1.DeploymentSpec{}, - v1beta1.DeploymentStatus{}, - } +// dep := &v1beta1.Deployment{ +// meta_v1.TypeMeta{}, +// meta_v1.ObjectMeta{ +// Name: "deployment-1", +// Namespace: "xx", +// Labels: map[string]string{types.KeelPolicyLabel: "all"}, +// }, +// v1beta1.DeploymentSpec{}, +// v1beta1.DeploymentStatus{}, +// } - grc := &k8s.GenericResourceCache{} - fp.podList = &v1.PodList{ - Items: []v1.Pod{ - v1.Pod{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "1", - Namespace: "xx", - }, - }, - v1.Pod{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "2", - Namespace: "xx", - }, - }, - }, - } +// grc := &k8s.GenericResourceCache{} +// fp.podList = &v1.PodList{ +// Items: []v1.Pod{ +// v1.Pod{ +// ObjectMeta: meta_v1.ObjectMeta{ +// Name: "1", +// Namespace: "xx", +// }, +// }, +// v1.Pod{ +// ObjectMeta: meta_v1.ObjectMeta{ +// Name: "2", +// Namespace: "xx", +// }, +// }, +// }, +// } - provider, err := NewProvider(fp, &fakeSender{}, approver(), grc) - if err != nil { - t.Fatalf("failed to get provider: %s", err) - } +// provider, err := NewProvider(fp, &fakeSender{}, approver(), grc) +// if err != nil { +// t.Fatalf("failed to get provider: %s", err) +// } - err = provider.forceUpdate(dep) - if err != nil { - t.Fatalf("failed to force update: %s", err) - } +// err = provider.forceUpdate(dep) +// if err != nil { +// t.Fatalf("failed to force update: %s", err) +// } - if len(fp.deletedPods) != 2 { - t.Errorf("expected to get 2 deleted pods") - } +// if len(fp.deletedPods) != 2 { +// t.Errorf("expected to get 2 deleted pods") +// } - if fp.deletedPods[0].Namespace != "xx" { - t.Errorf("wrong namespace: %s", fp.deletedPods[0].Namespace) - } - if fp.deletedPods[1].Namespace != "xx" { - t.Errorf("wrong namespace: %s", fp.deletedPods[1].Namespace) - } +// if fp.deletedPods[0].Namespace != "xx" { +// t.Errorf("wrong namespace: %s", fp.deletedPods[0].Namespace) +// } +// if fp.deletedPods[1].Namespace != "xx" { +// t.Errorf("wrong namespace: %s", fp.deletedPods[1].Namespace) +// } - if fp.deletedPods[0].Name != "1" { - t.Errorf("wrong name: %s", fp.deletedPods[0].Name) - } - if fp.deletedPods[1].Name != "2" { - t.Errorf("wrong name: %s", fp.deletedPods[1].Name) - } -} +// if fp.deletedPods[0].Name != "1" { +// t.Errorf("wrong name: %s", fp.deletedPods[0].Name) +// } +// if fp.deletedPods[1].Name != "2" { +// t.Errorf("wrong name: %s", fp.deletedPods[1].Name) +// } +// } -func TestForceUpdateDelay(t *testing.T) { +// func TestForceUpdateDelay(t *testing.T) { - fp := &fakeImplementer{} +// fp := &fakeImplementer{} - dep := &v1beta1.Deployment{ - meta_v1.TypeMeta{}, - meta_v1.ObjectMeta{ - Name: "deployment-1", - Namespace: "xx", - Labels: map[string]string{types.KeelPolicyLabel: "all"}, - Annotations: map[string]string{types.KeelPodDeleteDelay: "300"}, - }, - v1beta1.DeploymentSpec{}, - v1beta1.DeploymentStatus{}, - } +// dep := &v1beta1.Deployment{ +// meta_v1.TypeMeta{}, +// meta_v1.ObjectMeta{ +// Name: "deployment-1", +// Namespace: "xx", +// Labels: map[string]string{types.KeelPolicyLabel: "all"}, +// Annotations: map[string]string{types.KeelPodDeleteDelay: "300"}, +// }, +// v1beta1.DeploymentSpec{}, +// v1beta1.DeploymentStatus{}, +// } - fp.podList = &v1.PodList{ - Items: []v1.Pod{ - v1.Pod{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "1", - Namespace: "xx", - }, - }, - v1.Pod{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "2", - Namespace: "xx", - }, - }, - }, - } +// fp.podList = &v1.PodList{ +// Items: []v1.Pod{ +// v1.Pod{ +// ObjectMeta: meta_v1.ObjectMeta{ +// Name: "1", +// Namespace: "xx", +// }, +// }, +// v1.Pod{ +// ObjectMeta: meta_v1.ObjectMeta{ +// Name: "2", +// Namespace: "xx", +// }, +// }, +// }, +// } - grc := &k8s.GenericResourceCache{} - provider, err := NewProvider(fp, &fakeSender{}, approver(), grc) - if err != nil { - t.Fatalf("failed to get provider: %s", err) - } +// grc := &k8s.GenericResourceCache{} +// provider, err := NewProvider(fp, &fakeSender{}, approver(), grc) +// if err != nil { +// t.Fatalf("failed to get provider: %s", err) +// } - go func() { - err = provider.forceUpdate(dep) - if err != nil { - t.Fatalf("failed to force update: %s", err) - } - }() +// go func() { +// err = provider.forceUpdate(dep) +// if err != nil { +// t.Fatalf("failed to force update: %s", err) +// } +// }() - time.Sleep(100 * time.Millisecond) +// time.Sleep(100 * time.Millisecond) - if len(fp.deletedPods) != 1 { - t.Errorf("expected to get 1 deleted pods, another one should be delayed") - } +// if len(fp.deletedPods) != 1 { +// t.Errorf("expected to get 1 deleted pods, another one should be delayed") +// } - if fp.deletedPods[0].Namespace != "xx" { - t.Errorf("wrong namespace: %s", fp.deletedPods[0].Namespace) - } - if fp.deletedPods[0].Name != "1" { - t.Errorf("wrong name: %s", fp.deletedPods[0].Name) - } -} +// if fp.deletedPods[0].Namespace != "xx" { +// t.Errorf("wrong namespace: %s", fp.deletedPods[0].Namespace) +// } +// if fp.deletedPods[0].Name != "1" { +// t.Errorf("wrong name: %s", fp.deletedPods[0].Name) +// } +// } diff --git a/provider/kubernetes/kubernetes.go b/provider/kubernetes/kubernetes.go index 5a59a9e6..faf28bc5 100644 --- a/provider/kubernetes/kubernetes.go +++ b/provider/kubernetes/kubernetes.go @@ -49,9 +49,6 @@ const ProviderName = "kubernetes" var versionreg = regexp.MustCompile(`:[^:]*$`) -// annotation used to specify which image to force pull -const forceUpdateImageAnnotation = "keel.sh/update-image" - // GenericResourceCache an interface for generic resource cache. type GenericResourceCache interface { // Values returns a copy of the contents of the cache. @@ -185,74 +182,6 @@ func (p *Provider) TrackedImages() ([]*types.TrackedImage, error) { return trackedImages, nil } -// TrackedImages - get tracked images -// func (p *Provider) TrackedImages() ([]*types.TrackedImage, error) { -// var trackedImages []*types.TrackedImage - -// deploymentLists, err := p.deployments() -// if err != nil { -// return nil, err -// } - -// for _, deploymentList := range deploymentLists { -// for _, deployment := range deploymentList.Items { -// labels := deployment.GetLabels() - -// // ignoring unlabelled deployments -// policy := policies.GetPolicy(labels) -// if policy == types.PolicyTypeNone { -// continue -// } - -// annotations := deployment.GetAnnotations() -// schedule, ok := annotations[types.KeelPollScheduleAnnotation] -// if ok { -// _, err := cron.Parse(schedule) -// if err != nil { -// log.WithFields(log.Fields{ -// "error": err, -// "schedule": schedule, -// "deployment": deployment.Name, -// "namespace": deployment.Namespace, -// }).Error("provider.kubernetes: failed to parse poll schedule, setting default schedule") -// schedule = types.KeelPollDefaultSchedule -// } -// } else { -// schedule = types.KeelPollDefaultSchedule -// } - -// // trigger type, we only care for "poll" type triggers -// trigger := policies.GetTriggerPolicy(labels) - -// secrets := getImagePullSecrets(&deployment) - -// images := getImages(&deployment) -// for _, img := range images { -// ref, err := image.Parse(img) -// if err != nil { -// log.WithFields(log.Fields{ -// "error": err, -// "image": img, -// "namespace": deployment.Namespace, -// "name": deployment.Name, -// }).Error("provider.kubernetes: failed to parse image") -// continue -// } -// trackedImages = append(trackedImages, &types.TrackedImage{ -// Image: ref, -// PollSchedule: schedule, -// Trigger: trigger, -// Provider: ProviderName, -// Namespace: deployment.Namespace, -// Secrets: secrets, -// }) -// } -// } -// } - -// return trackedImages, nil -// } - func (p *Provider) startInternal() error { for { select { @@ -332,7 +261,7 @@ func (p *Provider) updateDeployments(plans []*UpdatePlan) (updated []*k8s.Generi resource.SetAnnotations(annotations) err = p.implementer.Update(resource) - kubernetesVersionedUpdatesCounter.With(prometheus.Labels{"deployment": fmt.Sprintf("%s/%s", resource.Namespace, resource.Name)}).Inc() + kubernetesVersionedUpdatesCounter.With(prometheus.Labels{resource.Kind(): fmt.Sprintf("%s/%s", resource.Namespace, resource.Name)}).Inc() // } if err != nil { log.WithFields(log.Fields{ @@ -423,6 +352,7 @@ func (p *Provider) createUpdatePlans(repo *types.Repository) ([]*UpdatePlan, err policy := policies.GetPolicy(labels) if policy == types.PolicyTypeNone { // skip + log.Infof("no policy defined, skipping: %s, labels: %s", resource.Identifier, labels) continue } diff --git a/provider/kubernetes/kubernetes_test.go b/provider/kubernetes/kubernetes_test.go index 1182912b..1743370f 100644 --- a/provider/kubernetes/kubernetes_test.go +++ b/provider/kubernetes/kubernetes_test.go @@ -16,6 +16,8 @@ import ( // "k8s.io/api/extensions/apps_v1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" core_v1 "k8s.io/client-go/kubernetes/typed/core/v1" + + log "github.com/sirupsen/logrus" ) type fakeProvider struct { @@ -217,14 +219,17 @@ func MustParseGR(obj interface{}) *k8s.GenericResource { return gr } -func MustParseGRS(objs ...interface{}) []*k8s.GenericResource { - grs := []*k8s.GenericResource{} - for obj := range objs { - gr, err := k8s.NewGenericResource(obj) +func MustParseGRS(objs []*apps_v1.Deployment) []*k8s.GenericResource { + grs := make([]*k8s.GenericResource, len(objs)) + for idx, obj := range objs { + var err error + var gr *k8s.GenericResource + gr, err = k8s.NewGenericResource(obj) + log.Infof("step 1: new gr parsed: %s, %s", gr.Identifier, gr.GetLabels()) if err != nil { panic(err) } - grs = append(grs, gr) + grs[idx] = gr } return grs } @@ -242,8 +247,8 @@ func TestGetImpacted(t *testing.T) { }, } - deps := []apps_v1.Deployment{ - apps_v1.Deployment{ + deps := []*apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -263,7 +268,7 @@ func TestGetImpacted(t *testing.T) { }, apps_v1.DeploymentStatus{}, }, - apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-2", @@ -287,6 +292,11 @@ func TestGetImpacted(t *testing.T) { grs := MustParseGRS(deps) grc := &k8s.GenericResourceCache{} + + for _, gr := range grs { + log.Infof("step 2:current resrouce: %s labels: %s", gr.Identifier, gr.GetLabels()) + } + grc.Add(grs...) provider, err := NewProvider(fp, &fakeSender{}, approver(), grc) @@ -306,7 +316,7 @@ func TestGetImpacted(t *testing.T) { } if len(plans) != 1 { - t.Errorf("expected to find 1 deployment update plan but found %d", len(plans)) + t.Fatalf("expected to find 1 deployment update plan but found %d", len(plans)) } found := false @@ -337,17 +347,22 @@ func TestProcessEvent(t *testing.T) { }, }, } - deps := []apps_v1.Deployment{ - apps_v1.Deployment{ + deps := []*apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "deployment-1", - Namespace: "xxxx", + Namespace: "ns-1", Labels: map[string]string{types.KeelPolicyLabel: "all"}, Annotations: map[string]string{}, }, apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -359,16 +374,52 @@ func TestProcessEvent(t *testing.T) { }, apps_v1.DeploymentStatus{}, }, - apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "deployment-2", - Namespace: "xxxx", + Namespace: "ns-2", Labels: map[string]string{"whatever": "all"}, Annotations: map[string]string{}, }, apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + v1.Container{ + Image: "gcr.io/v2-namespace/bye-world:1.1.1", + }, + }, + }, + }, + }, + apps_v1.DeploymentStatus{}, + }, + { + meta_v1.TypeMeta{}, + meta_v1.ObjectMeta{ + Name: "deployment-3", + Namespace: "ns-3", + Labels: map[string]string{ + "whatever": "all", + "foo": "bar", + }, + Annotations: map[string]string{ + "ann": "1", + }, + }, + apps_v1.DeploymentSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -402,6 +453,10 @@ func TestProcessEvent(t *testing.T) { t.Errorf("got error while processing event: %s", err) } + if fp.updated == nil { + t.Fatalf("resource was not updated") + } + if fp.updated.Containers()[0].Image != repo.Name+":"+repo.Tag { t.Errorf("expected to find a deployment with updated image but found: %s", fp.updated.Containers()[0].Image) } @@ -419,8 +474,8 @@ func TestProcessEventBuildNumber(t *testing.T) { }, }, } - deps := []apps_v1.Deployment{ - apps_v1.Deployment{ + deps := []*apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "deployment-1", @@ -481,8 +536,8 @@ func TestGetImpactedTwoContainersInSameDeployment(t *testing.T) { }, }, } - deps := []apps_v1.Deployment{ - apps_v1.Deployment{ + deps := []*apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -506,7 +561,7 @@ func TestGetImpactedTwoContainersInSameDeployment(t *testing.T) { }, apps_v1.DeploymentStatus{}, }, - apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-2", @@ -580,8 +635,8 @@ func TestGetImpactedTwoSameContainersInSameDeployment(t *testing.T) { }, }, } - deps := []apps_v1.Deployment{ - apps_v1.Deployment{ + deps := []*apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -605,7 +660,7 @@ func TestGetImpactedTwoSameContainersInSameDeployment(t *testing.T) { }, apps_v1.DeploymentStatus{}, }, - apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-2", @@ -680,8 +735,8 @@ func TestGetImpactedUntaggedImage(t *testing.T) { }, }, } - deps := []apps_v1.Deployment{ - apps_v1.Deployment{ + deps := []*apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -702,7 +757,7 @@ func TestGetImpactedUntaggedImage(t *testing.T) { }, apps_v1.DeploymentStatus{}, }, - apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-2", @@ -777,8 +832,8 @@ func TestGetImpactedUntaggedOneImage(t *testing.T) { }, }, } - deps := []apps_v1.Deployment{ - apps_v1.Deployment{ + deps := []*apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -799,7 +854,7 @@ func TestGetImpactedUntaggedOneImage(t *testing.T) { }, apps_v1.DeploymentStatus{}, }, - apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-2", @@ -875,8 +930,8 @@ func TestTrackedImages(t *testing.T) { }, }, } - deps := []apps_v1.Deployment{ - apps_v1.Deployment{ + deps := []*apps_v1.Deployment{ + { meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", diff --git a/provider/kubernetes/unversioned_updates.go b/provider/kubernetes/unversioned_updates.go index 19624f9e..ceeb3300 100644 --- a/provider/kubernetes/unversioned_updates.go +++ b/provider/kubernetes/unversioned_updates.go @@ -74,44 +74,25 @@ func (p *Provider) checkUnversionedDeployment(policy types.PolicyType, repo *typ // annotations := resource.GetAnnotations() matchTag, ok := annotations[types.KeelForceTagMatchLabel] if ok { - if matchTag == "true" && containerImageRef.Tag() != eventRepoRef.Tag() { + if matchTag != "" && containerImageRef.Tag() != eventRepoRef.Tag() { continue } - // if deployment.Spec.Template.Annotations == nil { - // deployment.Spec.Template.Annotations = map[string]string{} - // } - - // deployment.Spec.Template.Annotations["time"] = timeutil.Now().String() } + // updating spec template annotations + specAnnotations := resource.GetSpecAnnotations() + specAnnotations[types.KeelUpdateTimeAnnotation] = time.Now().String() + resource.SetSpecAnnotations(specAnnotations) + // updating image if containerImageRef.Registry() == image.DefaultRegistryHostname { - // c.Image = fmt.Sprintf("%s:%s", containerImageRef.ShortName(), repo.Tag) resource.UpdateContainer(idx, fmt.Sprintf("%s:%s", containerImageRef.ShortName(), repo.Tag)) } else { - // c.Image = fmt.Sprintf("%s:%s", containerImageRef.Repository(), repo.Tag) resource.UpdateContainer(idx, fmt.Sprintf("%s:%s", containerImageRef.Repository(), repo.Tag)) } - // deployment.Spec.Template.Spec.Containers[idx] = c - // marking this deployment for update shouldUpdateDeployment = true - // updating annotations - // annotations := resource.GetAnnotations() - // updating digest if available - // if repo.Digest != "" { - - // annotations[types.KeelDigestAnnotation+"/"+containerImageRef.Remote()] = repo.Digest - // } - - // adding image for updates - // annotations = addImageToPull(annotations, c.Image) - - annotations["keel.sh/update-time"] = time.Now().String() - - resource.SetAnnotations(annotations) - updatePlan.CurrentVersion = containerImageRef.Tag() updatePlan.NewVersion = repo.Tag updatePlan.Resource = resource diff --git a/provider/kubernetes/unversioned_updates_test.go b/provider/kubernetes/unversioned_updates_test.go index 392619c6..340a6c6f 100644 --- a/provider/kubernetes/unversioned_updates_test.go +++ b/provider/kubernetes/unversioned_updates_test.go @@ -59,6 +59,11 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { }, apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -77,11 +82,16 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { meta_v1.ObjectMeta{ Name: "dep-1", Namespace: "xxxx", - Annotations: map[string]string{forceUpdateImageAnnotation: "gcr.io/v2-namespace/hello-world:latest"}, + Annotations: map[string]string{}, Labels: map[string]string{types.KeelPolicyLabel: "all"}, }, apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -185,6 +195,11 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { }, apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -203,11 +218,16 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { meta_v1.ObjectMeta{ Name: "dep-1", Namespace: "xxxx", - Annotations: map[string]string{forceUpdateImageAnnotation: "karolisr/keel:0.2.0"}, + Annotations: map[string]string{}, Labels: map[string]string{types.KeelPolicyLabel: "force"}, }, apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -240,6 +260,11 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { }, apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -260,12 +285,16 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { Namespace: "xxxx", Annotations: map[string]string{ types.KeelPollScheduleAnnotation: types.KeelPollDefaultSchedule, - forceUpdateImageAnnotation: "karolisr/keel:master", }, Labels: map[string]string{types.KeelPolicyLabel: "force"}, }, apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -302,6 +331,11 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { }, apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -323,7 +357,6 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { Annotations: map[string]string{ types.KeelPollScheduleAnnotation: types.KeelPollDefaultSchedule, types.KeelForceTagMatchLabel: "yup", - forceUpdateImageAnnotation: "karolisr/keel:latest-staging", }, Labels: map[string]string{types.KeelPolicyLabel: "force"}, }, @@ -331,7 +364,7 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { Template: v1.PodTemplateSpec{ ObjectMeta: meta_v1.ObjectMeta{ Annotations: map[string]string{ - "time": timeutil.Now().String(), + "this": "that", }, }, Spec: v1.PodSpec{ @@ -396,7 +429,6 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { Annotations: map[string]string{ types.KeelPollScheduleAnnotation: types.KeelPollDefaultSchedule, types.KeelForceTagMatchLabel: "yup", - forceUpdateImageAnnotation: "eu.gcr.io/karolisr/keel:latest-staging", }, Labels: map[string]string{types.KeelPolicyLabel: "force"}, }, @@ -405,7 +437,7 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { ObjectMeta: meta_v1.ObjectMeta{ Annotations: map[string]string{ "this": "that", - "time": timeutil.Now().String(), + // "time": timeutil.Now().String(), }, }, Spec: v1.PodSpec{ @@ -440,6 +472,7 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { }, apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -473,6 +506,18 @@ func TestProvider_checkUnversionedDeployment(t *testing.T) { t.Errorf("Provider.checkUnversionedDeployment() error = %#v, wantErr %#v", err, tt.wantErr) return } + + if gotShouldUpdateDeployment { + ann := gotUpdatePlan.Resource.GetSpecAnnotations() + + if ann[types.KeelUpdateTimeAnnotation] != "" { + delete(ann, types.KeelUpdateTimeAnnotation) + gotUpdatePlan.Resource.SetSpecAnnotations(ann) + } else { + t.Errorf("Provider.checkUnversionedDeployment() missing types.KeelUpdateTimeAnnotation annotation") + } + } + if !reflect.DeepEqual(gotUpdatePlan, tt.wantUpdatePlan) { t.Errorf("Provider.checkUnversionedDeployment() gotUpdatePlan = %#v, want %#v", gotUpdatePlan, tt.wantUpdatePlan) } diff --git a/provider/kubernetes/versioned_updates.go b/provider/kubernetes/versioned_updates.go index e6b57b0a..ac3b74b2 100644 --- a/provider/kubernetes/versioned_updates.go +++ b/provider/kubernetes/versioned_updates.go @@ -74,33 +74,14 @@ func (p *Provider) checkVersionedDeployment(newVersion *types.Version, policy ty // if policy is force, don't bother with version checking // same with `latest` images, update them to versioned ones if policy == types.PolicyTypeForce || containerImageRef.Tag() == "latest" { - // c = updateContainer(c, conatinerImageRef, newVersion.String()) - if containerImageRef.Registry() == image.DefaultRegistryHostname { - // container.Image = fmt.Sprintf("%s:%s", ref.ShortName(), version) - resource.UpdateContainer(idx, fmt.Sprintf("%s:%s", containerImageRef.ShortName(), newVersion.String())) - } else { - // container.Image = fmt.Sprintf("%s:%s", ref.Repository(), version) - resource.UpdateContainer(idx, fmt.Sprintf("%s:%s", containerImageRef.Repository(), newVersion.String())) } - - // deployment.Spec.Template.Spec.Containers[idx] = c - - // marking this deployment for update shouldUpdateDeployment = true - // updating digest if available - annotations := resource.GetAnnotations() + setUpdateTime(resource) - annotations["keel.sh/update-time"] = time.Now().String() - // if repo.Digest != "" { - // annotations[types.KeelDigestAnnotation+"/"+conatinerImageRef.Remote()] = repo.Digest - // } - // annotations = addImageToPull(annotations, c.Image) - - resource.SetAnnotations(annotations) log.WithFields(log.Fields{ "parsed_image": containerImageRef.Remote(), "raw_image_name": c.Image, @@ -173,14 +154,7 @@ func (p *Provider) checkVersionedDeployment(newVersion *types.Version, policy ty // marking this deployment for update shouldUpdateDeployment = true - // updating annotations - annotations := resource.GetAnnotations() - // updating digest if available - // if repo.Digest != "" { - // annotations[types.KeelDigestAnnotation+"/"+conatinerImageRef.Remote()] = repo.Digest - // } - annotations["keel.sh/update-time"] = time.Now().String() - resource.SetAnnotations(annotations) + setUpdateTime(resource) updatePlan.CurrentVersion = currentVersion.Original updatePlan.NewVersion = newVersion.Original @@ -199,13 +173,8 @@ func (p *Provider) checkVersionedDeployment(newVersion *types.Version, policy ty return updatePlan, shouldUpdateDeployment, nil } -// func updateContainer(container v1.Container, ref *image.Reference, version string) v1.Container { -// // updating image -// if ref.Registry() == image.DefaultRegistryHostname { -// container.Image = fmt.Sprintf("%s:%s", ref.ShortName(), version) -// } else { -// container.Image = fmt.Sprintf("%s:%s", ref.Repository(), version) -// } - -// return container -// } +func setUpdateTime(resource *k8s.GenericResource) { + specAnnotations := resource.GetSpecAnnotations() + specAnnotations[types.KeelUpdateTimeAnnotation] = time.Now().String() + resource.SetSpecAnnotations(specAnnotations) +} diff --git a/provider/kubernetes/versioned_updates_test.go b/provider/kubernetes/versioned_updates_test.go index 6927cfb2..d58bba9c 100644 --- a/provider/kubernetes/versioned_updates_test.go +++ b/provider/kubernetes/versioned_updates_test.go @@ -6,13 +6,14 @@ import ( "github.com/keel-hq/keel/approvals" "github.com/keel-hq/keel/extension/notification" + "github.com/keel-hq/keel/internal/k8s" "github.com/keel-hq/keel/types" "github.com/keel-hq/keel/util/version" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apps_v1 "k8s.io/api/apps/v1" "k8s.io/api/core/v1" - "k8s.io/api/extensions/v1beta1" ) func unsafeGetVersion(ver string) *types.Version { @@ -35,7 +36,7 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { newVersion *types.Version policy types.PolicyType repo *types.Repository - deployment v1beta1.Deployment + resource *k8s.GenericResource } tests := []struct { name string @@ -51,7 +52,7 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { newVersion: unsafeGetVersion("1.1.2"), policy: types.PolicyTypeAll, repo: &types.Repository{Name: "gcr.io/v2-namespace/hello-world", Tag: "1.1.2"}, - deployment: v1beta1.Deployment{ + resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -59,8 +60,13 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { Annotations: map[string]string{}, Labels: map[string]string{types.KeelPolicyLabel: "all"}, }, - v1beta1.DeploymentSpec{ + apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -70,11 +76,11 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { }, }, }, - v1beta1.DeploymentStatus{}, - }, + apps_v1.DeploymentStatus{}, + }), }, wantUpdatePlan: &UpdatePlan{ - Deployment: v1beta1.Deployment{ + Resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -82,8 +88,13 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { Annotations: map[string]string{}, Labels: map[string]string{types.KeelPolicyLabel: "all"}, }, - v1beta1.DeploymentSpec{ + apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -93,8 +104,8 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { }, }, }, - v1beta1.DeploymentStatus{}, - }, + apps_v1.DeploymentStatus{}, + }), NewVersion: "1.1.2", CurrentVersion: "1.1.1", }, @@ -107,7 +118,7 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { newVersion: unsafeGetVersion("1.1.1"), policy: types.PolicyTypeAll, repo: &types.Repository{Name: "gcr.io/v2-namespace/hello-world", Tag: "1.1.1"}, - deployment: v1beta1.Deployment{ + resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -115,7 +126,7 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { Annotations: map[string]string{}, Labels: map[string]string{types.KeelPolicyLabel: "all"}, }, - v1beta1.DeploymentSpec{ + apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ Spec: v1.PodSpec{ Containers: []v1.Container{ @@ -126,11 +137,11 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { }, }, }, - v1beta1.DeploymentStatus{}, - }, + apps_v1.DeploymentStatus{}, + }), }, wantUpdatePlan: &UpdatePlan{ - Deployment: v1beta1.Deployment{}, + Resource: nil, NewVersion: "", CurrentVersion: "", }, @@ -143,7 +154,7 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { newVersion: unsafeGetVersion("1.1.2"), policy: types.PolicyTypeAll, repo: &types.Repository{Name: "gcr.io/v2-namespace/hello-world", Tag: "1.1.2"}, - deployment: v1beta1.Deployment{ + resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -151,8 +162,13 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { Annotations: map[string]string{}, Labels: map[string]string{types.KeelPolicyLabel: "all"}, }, - v1beta1.DeploymentSpec{ + apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -165,11 +181,11 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { }, }, }, - v1beta1.DeploymentStatus{}, - }, + apps_v1.DeploymentStatus{}, + }), }, wantUpdatePlan: &UpdatePlan{ - Deployment: v1beta1.Deployment{ + Resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -177,8 +193,13 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { Annotations: map[string]string{}, Labels: map[string]string{types.KeelPolicyLabel: "all"}, }, - v1beta1.DeploymentSpec{ + apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -191,8 +212,8 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { }, }, }, - v1beta1.DeploymentStatus{}, - }, + apps_v1.DeploymentStatus{}, + }), NewVersion: "1.1.2", CurrentVersion: "1.1.1", }, @@ -205,7 +226,7 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { newVersion: unsafeGetVersion("1.1.2"), policy: types.PolicyTypeForce, repo: &types.Repository{Name: "gcr.io/v2-namespace/hello-world", Tag: "1.1.2"}, - deployment: v1beta1.Deployment{ + resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", @@ -213,8 +234,13 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { Annotations: map[string]string{}, Labels: map[string]string{types.KeelPolicyLabel: "force"}, }, - v1beta1.DeploymentSpec{ + apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -227,20 +253,25 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { }, }, }, - v1beta1.DeploymentStatus{}, - }, + apps_v1.DeploymentStatus{}, + }), }, wantUpdatePlan: &UpdatePlan{ - Deployment: v1beta1.Deployment{ + Resource: MustParseGR(&apps_v1.Deployment{ meta_v1.TypeMeta{}, meta_v1.ObjectMeta{ Name: "dep-1", Namespace: "xxxx", - Annotations: map[string]string{forceUpdateImageAnnotation: "gcr.io/v2-namespace/hello-world:1.1.2"}, + Annotations: map[string]string{}, Labels: map[string]string{types.KeelPolicyLabel: "force"}, }, - v1beta1.DeploymentSpec{ + apps_v1.DeploymentSpec{ Template: v1.PodTemplateSpec{ + ObjectMeta: meta_v1.ObjectMeta{ + Annotations: map[string]string{ + "this": "that", + }, + }, Spec: v1.PodSpec{ Containers: []v1.Container{ v1.Container{ @@ -253,8 +284,8 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { }, }, }, - v1beta1.DeploymentStatus{}, - }, + apps_v1.DeploymentStatus{}, + }), NewVersion: "1.1.2", CurrentVersion: "latest", }, @@ -262,6 +293,7 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { wantErr: false, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { p := &Provider{ @@ -271,11 +303,23 @@ func TestProvider_checkVersionedDeployment(t *testing.T) { events: tt.fields.events, stop: tt.fields.stop, } - gotUpdatePlan, gotShouldUpdateDeployment, err := p.checkVersionedDeployment(tt.args.newVersion, tt.args.policy, tt.args.repo, tt.args.deployment) + gotUpdatePlan, gotShouldUpdateDeployment, err := p.checkVersionedDeployment(tt.args.newVersion, tt.args.policy, tt.args.repo, tt.args.resource) if (err != nil) != tt.wantErr { t.Errorf("Provider.checkVersionedDeployment() error = %v, wantErr %v", err, tt.wantErr) return } + + if gotShouldUpdateDeployment { + ann := gotUpdatePlan.Resource.GetSpecAnnotations() + _, ok := ann[types.KeelUpdateTimeAnnotation] + if ok { + delete(ann, types.KeelUpdateTimeAnnotation) + gotUpdatePlan.Resource.SetSpecAnnotations(ann) + } else { + t.Errorf("Provider.checkVersionedDeployment() missing types.KeelUpdateTimeAnnotation annotation") + } + } + if !reflect.DeepEqual(gotUpdatePlan, tt.wantUpdatePlan) { t.Errorf("Provider.checkVersionedDeployment() gotUpdatePlan = %v, want %v", gotUpdatePlan, tt.wantUpdatePlan) }