1. origin
  2. Commit 7d705af09388d8a46a9901ebe3ba28ffb992d695
Browse code

When creating build, lookup "to" field if specified

Also change builds to go into "Error" status when a problem occurs
and record a message. Slightly refactor the build controller code.
If the build pod already exists, assume that there is a race in build
execution and stay in "pending" state.

This means that clients that want to create ImageRepositories and
BuildConfigs at the same time that are using the integrated registry
only need to specify:

POST /imageRepositories {
"metadata": {
"name": "myimage",
},
}
POST /buildConfigs {
"parameters": {
"output": {
"to": {
"name": "myimage",
}
}
}
}

and the build will be configured with the right registry name automatically.
The integrated registry must be configured properly and OpenShift must be
configured to default to it via OPENSHIFT_DEFAULT_REGISTRY.

Clayton Coleman authored on 2015/01/20 14:00:41
Showing 4 changed files
pkg/build/api/types.go
History View file @ 7d705af
... ... 
@@ -16,6 +16,9 @@ type Build struct {
16 16 
 	// Status is the current status of the build.
17 17 
 	Status BuildStatus `json:"status,omitempty"`
18 18
19 
+	// A human readable message indicating details about why the build has this status
20 
+	Message string `json:"message,omitempty"`
21 
+
19 22 
 	// PodName is the name of the pod that is used to execute the build
20 23 
 	PodName string `json:"podName,omitempty"`
21 24
pkg/build/api/v1beta1/types.go
History View file @ 7d705af
... ... 
@@ -16,6 +16,9 @@ type Build struct {
16 16 
 	// Status is the current status of the build.
17 17 
 	Status BuildStatus `json:"status,omitempty"`
18 18
19 
+	// A human readable message indicating details about why the build has this status
20 
+	Message string `json:"message,omitempty"`
21 
+
19 22 
 	// PodName is the name of the pod that is used to execute the build
20 23 
 	PodName string `json:"podName,omitempty"`
21 24
pkg/build/controller/controller.go
History View file @ 7d705af
... ... 
@@ -11,6 +11,7 @@ import (
11 11 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
12 12
13 13 
 	buildapi "github.com/openshift/origin/pkg/build/api"
14 
+	image "github.com/openshift/origin/pkg/image/api"
14 15 
 )
15 16
16 17 
 // BuildController watches build resources and manages their state
... ... 
@@ -21,6 +22,8 @@ type BuildController struct {
21 21 
 	BuildUpdater  buildUpdater
22 22 
 	PodManager    podManager
23 23 
 	BuildStrategy BuildStrategy
24 
+
25 
+	ImageRepositoryClient imageRepositoryClient
24 26 
 }
25 27
26 28 
 // BuildStrategy knows how to create a pod spec for a pod which can execute a build.
... ... 
@@ -37,6 +40,10 @@ type podManager interface {
37 37 
 	DeletePod(namespace string, pod *kapi.Pod) error
38 38 
 }
39 39
40 
+type imageRepositoryClient interface {
41 
+	GetImageRepository(namespace, name string) (*image.ImageRepository, error)
42 
+}
43 
+
40 44 
 // Run begins watching and syncing build jobs onto the cluster.
41 45 
 func (bc *BuildController) Run() {
42 46 
 	go util.Forever(func() { bc.HandleBuild(bc.NextBuild()) }, 0)
... ... 
@@ -51,39 +58,77 @@ func (bc *BuildController) HandleBuild(build *buildapi.Build) {
51 51 
 		return
52 52 
 	}
53 53
54 
-	nextStatus := buildapi.BuildStatusFailed
55 
-	build.PodName = fmt.Sprintf("build-%s", build.Name)
54 
+	if err := bc.nextBuildStatus(build); err != nil {
55 
+		glog.V(4).Infof("Build failed with error %s/%s: %#v", build.Namespace, build.Name, err)
56 
+		build.Status = buildapi.BuildStatusError
57 
+		build.Message = err.Error()
58 
+	}
56 59
60 
+	if _, err := bc.BuildUpdater.UpdateBuild(build.Namespace, build); err != nil {
61 
+		glog.V(2).Infof("Failed to record changes to build %s/%s: %#v", build.Namespace, build.Name, err)
62 
+	}
63 
+}
64 
+
65 
+// nextBuildStatus updates build with any appropriate changes, or returns an error if
66 
+// the change cannot occur. When returning nil, be sure to set build.Status and optionally
67 
+// build.Message.
68 
+func (bc *BuildController) nextBuildStatus(build *buildapi.Build) error {
57 69 
 	// If a cancelling event was triggered for the build, update build status.
58 70 
 	if build.Cancelled {
59 
-		glog.V(2).Infof("Cancelling build %s.", build.Name)
60 
-		nextStatus = buildapi.BuildStatusCancelled
61 
-
62 
-	} else {
63 
-
64 
-		var podSpec *kapi.Pod
65 
-		var err error
66 
-		if podSpec, err = bc.BuildStrategy.CreateBuildPod(build); err != nil {
67 
-			glog.V(2).Infof("Strategy failed to create build pod definition: %v", err)
68 
-			nextStatus = buildapi.BuildStatusFailed
69 
-		} else {
70 
-
71 
-			if _, err := bc.PodManager.CreatePod(build.Namespace, podSpec); err != nil {
72 
-				if !errors.IsAlreadyExists(err) {
73 
-					glog.V(2).Infof("Failed to create pod for build %s: %#v", build.Name, err)
74 
-					nextStatus = buildapi.BuildStatusFailed
75 
-				}
76 
-			} else {
77 
-				glog.V(2).Infof("Created build pod: %#v", podSpec)
78 
-				nextStatus = buildapi.BuildStatusPending
71 
+		glog.V(4).Infof("Cancelling build %s.", build.Name)
72 
+		build.Status = buildapi.BuildStatusCancelled
73 
+		return nil
74 
+	}
75 
+
76 
+	// lookup the destination from the referenced image repository
77 
+	spec := build.Parameters.Output.DockerImageReference
78 
+	if ref := build.Parameters.Output.To; ref != nil {
79 
+		// TODO: security, ensure that the reference image stream is actually visible
80 
+		namespace := ref.Namespace
81 
+		if len(namespace) == 0 {
82 
+			namespace = build.Namespace
83 
+		}
84 
+
85 
+		repo, err := bc.ImageRepositoryClient.GetImageRepository(namespace, ref.Name)
86 
+		if err != nil {
87 
+			if errors.IsNotFound(err) {
88 
+				build.Status = buildapi.BuildStatusFailed
89 
+				build.Message = fmt.Sprintf("The referenced output image repository %s/%s does not exist", namespace, ref.Name)
90 
+				return nil
79 91 
 			}
92 
+			return fmt.Errorf("the referenced output repo %s/%s could not be found by %s/%s: %v", namespace, ref.Name, build.Namespace, build.Name, err)
80 93 
 		}
94 
+		spec = repo.Status.DockerImageRepository
81 95 
 	}
82 96
83 
-	build.Status = nextStatus
84 
-	if _, err := bc.BuildUpdater.UpdateBuild(build.Namespace, build); err != nil {
85 
-		glog.V(2).Infof("Failed to update build %s: %#v", build.Name, err)
97 
+	// set the expected build parameters, which will be saved if no error occurs
98 
+	build.Status = buildapi.BuildStatusPending
99 
+	build.PodName = fmt.Sprintf("build-%s", build.Name)
100 
+
101 
+	// override DockerImageReference in the strategy for the copy we send to the server
102 
+	copy, err := kapi.Scheme.Copy(build)
103 
+	if err != nil {
104 
+		return fmt.Errorf("unable to copy build: %v", err)
105 
+	}
106 
+	buildCopy := copy.(*buildapi.Build)
107 
+	buildCopy.Parameters.Output.DockerImageReference = spec
108 
+
109 
+	// invoke the strategy to get a build pod
110 
+	podSpec, err := bc.BuildStrategy.CreateBuildPod(buildCopy)
111 
+	if err != nil {
112 
+		return fmt.Errorf("the strategy failed to create a build pod for %s/%s: %v", build.Namespace, build.Name, err)
113 
+	}
114 
+
115 
+	if _, err := bc.PodManager.CreatePod(build.Namespace, podSpec); err != nil {
116 
+		if errors.IsAlreadyExists(err) {
117 
+			glog.V(4).Infof("Build pod already existed: %#v", podSpec)
118 
+			return nil
119 
+		}
120 
+		return fmt.Errorf("failed to create pod for build %s/%s: s", build.Namespace, build.Name, err)
86 121 
 	}
122 
+
123 
+	glog.V(4).Infof("Created pod for build: %#v", podSpec)
124 
+	return nil
87 125 
 }
88 126
89 127 
 func (bc *BuildController) HandlePod(pod *kapi.Pod) {
pkg/build/controller/controller_test.go
History View file @ 7d705af
... ... 
@@ -9,6 +9,7 @@ import (
9 9
10 10 
 	buildapi "github.com/openshift/origin/pkg/build/api"
11 11 
 	buildtest "github.com/openshift/origin/pkg/build/controller/test"
12 
+	imageapi "github.com/openshift/origin/pkg/image/api"
12 13 
 )
13 14
14 15 
 type okBuildUpdater struct{}
... ... 
@@ -23,9 +24,12 @@ func (ebu *errBuildUpdater) UpdateBuild(namespace string, build *buildapi.Build)
23 23 
 	return &buildapi.Build{}, errors.New("UpdateBuild error!")
24 24 
 }
25 25
26 
-type okStrategy struct{}
26 
+type okStrategy struct {
27 
+	build *buildapi.Build
28 
+}
27 29
28 30 
 func (os *okStrategy) CreateBuildPod(build *buildapi.Build) (*kapi.Pod, error) {
31 
+	os.build = build
29 32 
 	return &kapi.Pod{}, nil
30 33 
 }
31 34
... ... 
@@ -65,10 +69,34 @@ func (_ *errExistsPodManager) DeletePod(namespace string, pod *kapi.Pod) error {
65 65 
 	return kerrors.NewNotFound("kind", "name")
66 66 
 }
67 67
68 
-func mockBuildAndController(status buildapi.BuildStatus) (build *buildapi.Build, controller *BuildController) {
68 
+type okImageRepositoryClient struct{}
69 
+
70 
+func (_ *okImageRepositoryClient) GetImageRepository(namespace, name string) (*imageapi.ImageRepository, error) {
71 
+	return &imageapi.ImageRepository{
72 
+		ObjectMeta: kapi.ObjectMeta{Name: name, Namespace: namespace},
73 
+		Status: imageapi.ImageRepositoryStatus{
74 
+			DockerImageRepository: "image/repo",
75 
+		},
76 
+	}, nil
77 
+}
78 
+
79 
+type errImageRepositoryClient struct{}
80 
+
81 
+func (_ *errImageRepositoryClient) GetImageRepository(namespace, name string) (*imageapi.ImageRepository, error) {
82 
+	return nil, errors.New("GetImageRepository error!")
83 
+}
84 
+
85 
+type errNotFoundImageRepositoryClient struct{}
86 
+
87 
+func (_ *errNotFoundImageRepositoryClient) GetImageRepository(namespace, name string) (*imageapi.ImageRepository, error) {
88 
+	return nil, kerrors.NewNotFound("ImageRepository", name)
89 
+}
90 
+
91 
+func mockBuildAndController(status buildapi.BuildStatus, output buildapi.BuildOutput) (build *buildapi.Build, controller *BuildController) {
69 92 
 	build = &buildapi.Build{
70 93 
 		ObjectMeta: kapi.ObjectMeta{
71 
-			Name: "dataBuild",
94 
+			Name:      "data-build",
95 
+			Namespace: "namespace",
72 96 
 			Labels: map[string]string{
73 97 
 				"name": "dataBuild",
74 98 
 			},
... ... 
@@ -86,20 +114,19 @@ func mockBuildAndController(status buildapi.BuildStatus) (build *buildapi.Build,
86 86 
 					ContextDir: "contextimage",
87 87 
 				},
88 88 
 			},
89 
-			Output: buildapi.BuildOutput{
90 
-				DockerImageReference: "repository/dataBuild",
91 
-			},
89 
+			Output: output,
92 90 
 		},
93 91 
 		Status:  status,
94 92 
 		PodName: "-the-pod-id",
95 93 
 	}
96 94 
 	controller = &BuildController{
97 
-		BuildStore:    buildtest.NewFakeBuildStore(build),
98 
-		BuildUpdater:  &okBuildUpdater{},
99 
-		PodManager:    &okPodManager{},
100 
-		NextBuild:     func() *buildapi.Build { return nil },
101 
-		NextPod:       func() *kapi.Pod { return nil },
102 
-		BuildStrategy: &okStrategy{},
95 
+		BuildStore:            buildtest.NewFakeBuildStore(build),
96 
+		BuildUpdater:          &okBuildUpdater{},
97 
+		PodManager:            &okPodManager{},
98 
+		NextBuild:             func() *buildapi.Build { return nil },
99 
+		NextPod:               func() *kapi.Pod { return nil },
100 
+		BuildStrategy:         &okStrategy{},
101 
+		ImageRepositoryClient: &okImageRepositoryClient{},
103 102 
 	}
104 103 
 	return
105 104 
 }
... ... 
@@ -124,60 +151,134 @@ func TestHandleBuild(t *testing.T) {
124 124 
 	type handleBuildTest struct {
125 125 
 		inStatus      buildapi.BuildStatus
126 126 
 		outStatus     buildapi.BuildStatus
127 
+		buildOutput   buildapi.BuildOutput
127 128 
 		buildStrategy BuildStrategy
128 129 
 		buildUpdater  buildUpdater
130 
+		imageClient   imageRepositoryClient
129 131 
 		podManager    podManager
132 
+		outputSpec    string
130 133 
 	}
131 134
132 135 
 	tests := []handleBuildTest{
133 136 
 		{ // 0
134 137 
 			inStatus:  buildapi.BuildStatusNew,
135 138 
 			outStatus: buildapi.BuildStatusPending,
139 
+			buildOutput: buildapi.BuildOutput{
140 
+				DockerImageReference: "repository/dataBuild",
141 
+			},
136 142 
 		},
137 143 
 		{ // 1
138 144 
 			inStatus:  buildapi.BuildStatusPending,
139 145 
 			outStatus: buildapi.BuildStatusPending,
146 
+			buildOutput: buildapi.BuildOutput{
147 
+				DockerImageReference: "repository/dataBuild",
148 
+			},
140 149 
 		},
141 150 
 		{ // 2
142 151 
 			inStatus:  buildapi.BuildStatusRunning,
143 152 
 			outStatus: buildapi.BuildStatusRunning,
153 
+			buildOutput: buildapi.BuildOutput{
154 
+				DockerImageReference: "repository/dataBuild",
155 
+			},
144 156 
 		},
145 157 
 		{ // 3
146 158 
 			inStatus:  buildapi.BuildStatusComplete,
147 159 
 			outStatus: buildapi.BuildStatusComplete,
160 
+			buildOutput: buildapi.BuildOutput{
161 
+				DockerImageReference: "repository/dataBuild",
162 
+			},
148 163 
 		},
149 164 
 		{ // 4
150 165 
 			inStatus:  buildapi.BuildStatusFailed,
151 166 
 			outStatus: buildapi.BuildStatusFailed,
167 
+			buildOutput: buildapi.BuildOutput{
168 
+				DockerImageReference: "repository/dataBuild",
169 
+			},
152 170 
 		},
153 171 
 		{ // 5
154 172 
 			inStatus:  buildapi.BuildStatusError,
155 173 
 			outStatus: buildapi.BuildStatusError,
174 
+			buildOutput: buildapi.BuildOutput{
175 
+				DockerImageReference: "repository/dataBuild",
176 
+			},
156 177 
 		},
157 178 
 		{ // 6
158 179 
 			inStatus:      buildapi.BuildStatusNew,
159 
-			outStatus:     buildapi.BuildStatusFailed,
180 
+			outStatus:     buildapi.BuildStatusError,
160 181 
 			buildStrategy: &errStrategy{},
182 
+			buildOutput: buildapi.BuildOutput{
183 
+				DockerImageReference: "repository/dataBuild",
184 
+			},
161 185 
 		},
162 186 
 		{ // 7
163 187 
 			inStatus:   buildapi.BuildStatusNew,
164 
-			outStatus:  buildapi.BuildStatusFailed,
188 
+			outStatus:  buildapi.BuildStatusError,
165 189 
 			podManager: &errPodManager{},
190 
+			buildOutput: buildapi.BuildOutput{
191 
+				DockerImageReference: "repository/dataBuild",
192 
+			},
166 193 
 		},
167 194 
 		{ // 8
168 195 
 			inStatus:   buildapi.BuildStatusNew,
169 
-			outStatus:  buildapi.BuildStatusFailed,
196 
+			outStatus:  buildapi.BuildStatusPending,
170 197 
 			podManager: &errExistsPodManager{},
198 
+			buildOutput: buildapi.BuildOutput{
199 
+				DockerImageReference: "repository/dataBuild",
200 
+			},
171 201 
 		},
172 202 
 		{ // 9
173 203 
 			inStatus:     buildapi.BuildStatusNew,
174 204 
 			outStatus:    buildapi.BuildStatusPending,
175 205 
 			buildUpdater: &errBuildUpdater{},
206 
+			buildOutput: buildapi.BuildOutput{
207 
+				DockerImageReference: "repository/dataBuild",
208 
+			},
209 
+		},
210 
+		{ // 10
211 
+			inStatus:  buildapi.BuildStatusNew,
212 
+			outStatus: buildapi.BuildStatusPending,
213 
+			buildOutput: buildapi.BuildOutput{
214 
+				To: &kapi.ObjectReference{
215 
+					Name: "foo",
216 
+				},
217 
+			},
218 
+			outputSpec: "image/repo",
219 
+		},
220 
+		{ // 11
221 
+			inStatus:  buildapi.BuildStatusNew,
222 
+			outStatus: buildapi.BuildStatusPending,
223 
+			buildOutput: buildapi.BuildOutput{
224 
+				To: &kapi.ObjectReference{
225 
+					Name:      "foo",
226 
+					Namespace: "bar",
227 
+				},
228 
+			},
229 
+			outputSpec: "image/repo",
230 
+		},
231 
+		{ // 12
232 
+			inStatus:    buildapi.BuildStatusNew,
233 
+			outStatus:   buildapi.BuildStatusFailed,
234 
+			imageClient: &errNotFoundImageRepositoryClient{},
235 
+			buildOutput: buildapi.BuildOutput{
236 
+				To: &kapi.ObjectReference{
237 
+					Name: "foo",
238 
+				},
239 
+			},
240 
+		},
241 
+		{ // 13
242 
+			inStatus:    buildapi.BuildStatusNew,
243 
+			outStatus:   buildapi.BuildStatusError,
244 
+			imageClient: &errImageRepositoryClient{},
245 
+			buildOutput: buildapi.BuildOutput{
246 
+				To: &kapi.ObjectReference{
247 
+					Name: "foo",
248 
+				},
249 
+			},
176 250 
 		},
177 251 
 	}
178 252
179 253 
 	for i, tc := range tests {
180 
-		build, ctrl := mockBuildAndController(tc.inStatus)
254 
+		build, ctrl := mockBuildAndController(tc.inStatus, tc.buildOutput)
181 255 
 		if tc.buildStrategy != nil {
182 256 
 			ctrl.BuildStrategy = tc.buildStrategy
183 257 
 		}
... ... 
@@ -187,12 +288,24 @@ func TestHandleBuild(t *testing.T) {
187 187 
 		if tc.podManager != nil {
188 188 
 			ctrl.PodManager = tc.podManager
189 189 
 		}
190 
+		if tc.imageClient != nil {
191 
+			ctrl.ImageRepositoryClient = tc.imageClient
192 
+		}
190 193
191 194 
 		ctrl.HandleBuild(build)
192 195
193 196 
 		if build.Status != tc.outStatus {
194 197 
 			t.Errorf("(%d) Expected %s, got %s!", i, tc.outStatus, build.Status)
195 198 
 		}
199 
+		if tc.inStatus != buildapi.BuildStatusError && build.Status == buildapi.BuildStatusError && len(build.Message) == 0 {
200 
+			t.Errorf("(%d) errored build should set message: %#v", i, build)
201 
+		}
202 
+		if len(tc.outputSpec) != 0 {
203 
+			build := ctrl.BuildStrategy.(*okStrategy).build
204 
+			if build.Parameters.Output.DockerImageReference != tc.outputSpec {
205 
+				t.Errorf("(%d) expected build sent to strategy to have docker spec %q: %#v", i, tc.outputSpec, build)
206 
+			}
207 
+		}
196 208 
 	}
197 209 
 }
198 210
... ... 
@@ -253,7 +366,7 @@ func TestHandlePod(t *testing.T) {
253 253 
 	}
254 254
255 255 
 	for i, tc := range tests {
256 
-		build, ctrl := mockBuildAndController(tc.inStatus)
256 
+		build, ctrl := mockBuildAndController(tc.inStatus, buildapi.BuildOutput{})
257 257 
 		pod := mockPod(tc.podStatus, tc.exitCode)
258 258 
 		if tc.matchID {
259 259 
 			build.PodName = pod.Name
... ... 
@@ -311,7 +424,7 @@ func TestCancelBuild(t *testing.T) {
311 311 
 	}
312 312
313 313 
 	for i, tc := range tests {
314 
-		build, ctrl := mockBuildAndController(tc.inStatus)
314 
+		build, ctrl := mockBuildAndController(tc.inStatus, buildapi.BuildOutput{})
315 315 
 		pod := mockPod(tc.podStatus, tc.exitCode)
316 316
317 317 
 		ctrl.CancelBuild(build, pod)