Browse code

Review 1: Comments and bashisms

Clayton Coleman authored on 2016/12/07 00:54:16
Showing 2 changed files
... ...
@@ -230,8 +230,8 @@ func (*dryRunRegistryPinger) ping(registry string) error {
230 230
 //
231 231
 // The ImageDeleter performs the following logic:
232 232
 //
233
-// remove any image was created at least *n* minutes ago and is *not* currently
234
-// referenced by:
233
+// remove any image that was created at least *n* minutes ago and is *not*
234
+// currently referenced by:
235 235
 //
236 236
 // - any pod created less than *n* minutes ago
237 237
 // - any image stream created less than *n* minutes ago
... ...
@@ -260,7 +260,7 @@ func NewPruner(options PrunerOptions) Pruner {
260 260
 	if options.PruneOverSizeLimit != nil {
261 261
 		pruneOverSizeLimit = fmt.Sprintf("%v", *options.PruneOverSizeLimit)
262 262
 	}
263
-	glog.V(1).Infof("Creating image pruner with keepYoungerThan=%v, keepTagRevisions=%s, pruneOverSizeLimit=%s allImages=%t",
263
+	glog.V(1).Infof("Creating image pruner with keepYoungerThan=%v, keepTagRevisions=%s, pruneOverSizeLimit=%s, allImages=%t",
264 264
 		options.KeepYoungerThan, keepTagRevisions, pruneOverSizeLimit, options.AllImages)
265 265
 
266 266
 	algorithm := pruneAlgorithm{}
... ...
@@ -314,11 +314,7 @@ func addImagesToGraph(g graph.Graph, images *imageapi.ImageList, algorithm prune
314 314
 		glog.V(4).Infof("Examining image %q", image.Name)
315 315
 
316 316
 		if !algorithm.allImages {
317
-			if image.Annotations == nil {
318
-				glog.V(4).Infof("Image %q with DockerImageReference %q belongs to an external registry - skipping", image.Name, image.DockerImageReference)
319
-				continue
320
-			}
321
-			if value, ok := image.Annotations[imageapi.ManagedByOpenShiftAnnotation]; !ok || value != "true" {
317
+			if image.Annotations[imageapi.ManagedByOpenShiftAnnotation] != "true" {
322 318
 				glog.V(4).Infof("Image %q with DockerImageReference %q belongs to an external registry - skipping", image.Name, image.DockerImageReference)
323 319
 				continue
324 320
 			}
... ...
@@ -105,7 +105,7 @@ os::cmd::try_until_success "curl --max-time 2 --fail --silent 'http://${DOCKER_R
105 105
 os::cmd::expect_success "curl -f http://${DOCKER_REGISTRY}/healthz"
106 106
 
107 107
 os::cmd::expect_success "dig @${API_HOST} docker-registry.default.local. A"
108
-registry_pod=$(oc get pod -n default -l deploymentconfig=docker-registry --template='{{(index .items 0).metadata.name}}')
108
+registry_pod="$(oc get pod -n default -l deploymentconfig=docker-registry --template='{{(index .items 0).metadata.name}}')"
109 109
 
110 110
 # Client setup (log in as e2e-user and set 'test' as the default project)
111 111
 # This is required to be able to push to the registry!
... ...
@@ -148,11 +148,11 @@ os::log::info "Ruby's testing blob digest: $rubyimageblob"
148 148
 os::log::info "Docker pullthrough"
149 149
 os::cmd::expect_success "oc import-image --confirm --from=mysql:latest mysql:pullthrough"
150 150
 os::cmd::expect_success "docker pull ${DOCKER_REGISTRY}/cache/mysql:pullthrough"
151
-mysqlblob="$(oc get istag -o go-template='{{range .image.dockerImageLayers}}{{if gt .size 1024.}}{{.name}},{{end}}{{end}}' "mysql:pullthrough" | cut -d , -f 1)"
151
+mysqlblob="$(oc get istag -o go-template='{{range .image.dockerImageLayers}}{{if gt .size 4096.}}{{.name}},{{end}}{{end}}' "mysql:pullthrough" | cut -d , -f 1)"
152 152
 # directly hit the image to trigger mirroring in case the layer already exists on disk
153 153
 os::cmd::expect_success "curl -H 'Authorization: bearer $(oc whoami -t)' 'http://${DOCKER_REGISTRY}/v2/cache/mysql/blobs/${mysqlblob}' 1>/dev/null"
154 154
 # verify the blob exists on disk in the registry due to mirroring under .../blobs/sha256/<2 char prefix>/<sha value>
155
-os::cmd::try_until_success "oc exec --context='${CLUSTER_ADMIN_CONTEXT}' -n default -p ${registry_pod} du /registry | tee '${LOG_DIR}/registry-images.txt' | grep '${mysqlblob:7:100}' | grep blobs"
155
+os::cmd::try_until_success "oc exec --context='${CLUSTER_ADMIN_CONTEXT}' -n default -p '${registry_pod}' du /registry | tee '${LOG_DIR}/registry-images.txt' | grep '${mysqlblob:7:100}' | grep blobs"
156 156
 
157 157
 os::log::info "Docker registry start with GCS"
158 158
 os::cmd::expect_failure_and_text "docker run -e REGISTRY_STORAGE=\"gcs: {}\" openshift/origin-docker-registry:${TAG}" "No bucket parameter provided"
... ...
@@ -533,7 +533,7 @@ os::cmd::expect_success 'oadm policy add-cluster-role-to-user system:image-prune
533 533
 os::cmd::try_until_text 'oadm policy who-can list images' 'system:serviceaccount:cache:builder'
534 534
 
535 535
 # run image pruning
536
-os::cmd::expect_success_and_not_text "oadm prune images --token=$(oc sa get-token builder -n cache) --keep-younger-than=0 --keep-tag-revisions=1 --confirm" 'error'
536
+os::cmd::expect_success_and_not_text "oadm prune images --token='$(oc sa get-token builder -n cache)' --keep-younger-than=0 --keep-tag-revisions=1 --confirm" 'error'
537 537
 
538 538
 # record the storage after pruning
539 539
 os::cmd::expect_success "oc exec -p ${registry_pod} du /registry > '${LOG_DIR}/prune-images.after.txt'"
... ...
@@ -543,16 +543,17 @@ os::cmd::expect_code "diff ${LOG_DIR}/prune-images.before.txt ${LOG_DIR}/prune-i
543 543
 
544 544
 # prune a mirror, external image that is no longer referenced
545 545
 os::cmd::expect_success "oc import-image nginx --confirm -n cache"
546
-nginxblob="$(oc get istag -o go-template='{{range .image.dockerImageLayers}}{{if gt .size 1024.}}{{.name}},{{end}}{{end}}' "nginx:latest" -n cache | cut -d , -f 1)"
546
+nginxblob="$(oc get istag -o go-template='{{range .image.dockerImageLayers}}{{if gt .size 4096.}}{{.name}},{{end}}{{end}}' "nginx:latest" -n cache | cut -d , -f 1)"
547 547
 # directly hit the image to trigger mirroring in case the layer already exists on disk
548 548
 os::cmd::expect_success "curl -H 'Authorization: bearer $(oc sa get-token builder -n cache)' 'http://${DOCKER_REGISTRY}/v2/cache/nginx/blobs/${nginxblob}' 1>/dev/null"
549 549
 # verify the blob exists on disk in the registry due to mirroring under .../blobs/sha256/<2 char prefix>/<sha value>
550
-os::cmd::try_until_success "oc exec --context='${CLUSTER_ADMIN_CONTEXT}' -n default -p ${registry_pod} du /registry | tee '${LOG_DIR}/registry-images.txt' | grep '${nginxblob:7:100}' | grep blobs"
550
+os::cmd::try_until_success "oc exec --context='${CLUSTER_ADMIN_CONTEXT}' -n default -p '${registry_pod}' du /registry | tee '${LOG_DIR}/registry-images.txt' | grep '${nginxblob:7:100}' | grep blobs"
551 551
 os::cmd::expect_success "oc delete is nginx -n cache"
552 552
 os::cmd::expect_success "oc exec -p ${registry_pod} du /registry > '${LOG_DIR}/prune-images.before.txt'"
553
-os::cmd::expect_success_and_not_text "oadm prune images --token=$(oc sa get-token builder -n cache) --keep-younger-than=0 --confirm --all --registry-url=${DOCKER_REGISTRY}" 'error'
553
+os::cmd::expect_success_and_not_text "oadm prune images --token='$(oc sa get-token builder -n cache)' --keep-younger-than=0 --confirm --all --registry-url='${DOCKER_REGISTRY}'" 'error'
554
+os::cmd::expect_failure "oc exec --context='${CLUSTER_ADMIN_CONTEXT}' -n default -p '${registry_pod}' du /registry | tee '${LOG_DIR}/registry-images.txt' | grep '${nginxblob:7:100}' | grep blobs"
554 555
 os::cmd::expect_success "oc exec -p ${registry_pod} du /registry > '${LOG_DIR}/prune-images.after.txt'"
555
-os::cmd::expect_code "diff ${LOG_DIR}/prune-images.before.txt ${LOG_DIR}/prune-images.after.txt" 1
556
+os::cmd::expect_code "diff '${LOG_DIR}/prune-images.before.txt' '${LOG_DIR}/prune-images.after.txt'" 1
556 557
 os::log::info "Validated image pruning"
557 558
 
558 559
 # with registry's re-deployment we loose all the blobs stored in its storage until now