From dac01fe6b9605997dba86ee24a85b113e49d408d Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Thu, 10 Mar 2016 22:53:48 +0100 Subject: [PATCH] WIP checkCircularReference + unit tests WIP Improve new-build output cycle detection This is a follow up to #7936. The idea is to test all combinations of build strategy/from, triggers and output reference, and improve the implementation to behave correctly in all cases. --- pkg/generate/app/cmd/newapp.go | 14 +++++++ pkg/generate/app/cmd/newapp_test.go | 62 +++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/pkg/generate/app/cmd/newapp.go b/pkg/generate/app/cmd/newapp.go index 46ac97eabdea..2be04b89ef81 100644 --- a/pkg/generate/app/cmd/newapp.go +++ b/pkg/generate/app/cmd/newapp.go @@ -717,6 +717,20 @@ func (c *AppConfig) checkCircularReferences(objects app.Objects) error { return nil } +// checkCircularReference returns an error if the output of the BuildConfig bc +// matches the build input and would automatically trigger build after build in +// a loop through an ImageChange trigger. +func checkCircularReference(bc *buildapi.BuildConfig) error { + if bc == nil { + return nil + } + input := buildutil.GetInputReference(bc.Spec.Strategy) + if bc.Spec.Output.To != nil && input != nil && reflect.DeepEqual(input, bc.Spec.Output.To) { + return app.CircularOutputReferenceError{Reference: fmt.Sprintf("%s", input.Name)} + } + return nil +} + func (c *AppConfig) Querying() bool { return c.AsList || c.AsSearch } diff --git a/pkg/generate/app/cmd/newapp_test.go b/pkg/generate/app/cmd/newapp_test.go index 874377024c58..2aaf496ca272 100644 --- a/pkg/generate/app/cmd/newapp_test.go +++ b/pkg/generate/app/cmd/newapp_test.go @@ -15,6 +15,7 @@ import ( "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util/sets" + buildapi "github.com/openshift/origin/pkg/build/api" client "github.com/openshift/origin/pkg/client/testclient" "github.com/openshift/origin/pkg/generate/app" templateapi "github.com/openshift/origin/pkg/template/api" @@ -400,3 +401,64 @@ func TestBuildPipelinesWithUnresolvedImage(t *testing.T) { t.Errorf("Expected ports=%v, got %v", e, a) } } + +func TestCheckCircularReference(t *testing.T) { + tests := []struct { + name string + bc *buildapi.BuildConfig + err error + }{ + { + name: "no nil pointer dereference", + bc: nil, + err: nil, + }, + { + name: "no output == no circular reference", + bc: &buildapi.BuildConfig{}, + err: nil, + }, + { + name: "no triggers == no build loop", + bc: &buildapi.BuildConfig{ + Spec: buildapi.BuildConfigSpec{ + Triggers: []buildapi.BuildTriggerPolicy{}, + BuildSpec: buildapi.BuildSpec{}, + }, + }, + err: nil, + }, + { + name: "ImageChange trigger + (input == output) == error", + bc: &buildapi.BuildConfig{ + Spec: buildapi.BuildConfigSpec{ + Triggers: []buildapi.BuildTriggerPolicy{ + {Type: buildapi.ImageChangeBuildTriggerType}, + }, + BuildSpec: buildapi.BuildSpec{ + Strategy: buildapi.BuildStrategy{ + SourceStrategy: &buildapi.SourceBuildStrategy{ + From: kapi.ObjectReference{ + Kind: "ImageStreamTag", + Name: "centos:latest", + }, + }, + }, + Output: buildapi.BuildOutput{ + To: &kapi.ObjectReference{ + Kind: "ImageStreamTag", + Name: "centos:latest", + }, + }, + }, + }, + }, + err: app.CircularOutputReferenceError{Reference: "centos:latest"}, + }, + } + for _, tt := range tests { + if got := checkCircularReference(tt.bc); got != tt.err { + t.Errorf("test: %s\ncheckCircularReference(bc) = %v, want %v", tt.name, got, tt.err) + } + } +}