Allow objects to request resolution, not just images #14795

smarterclayton · 2017-06-21T04:30:13Z

A lot of times, you want to be able to reuse a bit of config that works
fine elsewhere. By adding the alpha.image.policy.openshift.io/resolve-names
annotation with value * to the object you can request that resolution
attempt to match all image references to an image stream tag in the
current namespace, regardless of their own policy. This makes it easy to
transition to looking up image stream tags by name.

Extend oc set image-lookup to accept deploy/foo and to be able to
list and display non image stream resources.

@soltysh @mfojtik [test] this is somewhat last minute, but after using this
more I also wanted to be able to easily flip from using a config template
someone gave me to an image I built in the current namespace, with minimal
pain:

$ oc create -f SOMEONES_KUBE_OBJECT
// hack hack hack, wish i could build an image for hub/foo/bar:latest for this
$ oc new-build somerepo --output=bar:latest
$ oc set image-lookup deploy/mydeploy
// mydeploy magically picks up bar:latest from the image stream

soltysh

I'm additionally missing test-cmd for the oc image-lookup command itself.

soltysh · 2017-06-21T15:14:47Z

pkg/cmd/cli/cmd/set/imagelookup.go

+		namespace for any image that matches, regardless of whether the image stream has lookup
+		enabled.
+
+		    $ %[1]s run mysql --image=myregistry:5000/test/mysql:v1


Mind fixing the formatting here.

soltysh · 2017-06-21T15:21:48Z

pkg/cmd/cli/cmd/set/imagelookup.go

+		default:
+			accessor, ok := ometa.GetAnnotationAccessor(info.Object)
+			if !ok {
+				fmt.Fprintf(w, "%s/%s\tUNKNOWN\n", info.Mapping.Resource, info.Name)


Make this error message more readable, UNKNOWN is not helpful.

This method allows returning errors, but you're not actually using it anywhere, it would be good to either gather errors in a list and return aggregate, or return at any error. I'd personally went with the former.

This isn't an error message, this is indicating that the provided object doesn't support the annotation. We don't display errors in list views usually, we display something simple. Effectively, this is "unsupported".

Having resource/X is not supported seems more understandable in that case.

soltysh · 2017-06-21T15:27:43Z

test/extended/images/resolve.go

+		o.Expect(err).NotTo(o.HaveOccurred())
+
+		if pod.Spec.Containers[0].Image != tag.Image.DockerImageReference {
+			g.Skip("default image resolution is not configured, can't verify pod resolution")


This worries me a bit, why do we want to have this skip?

Because admins can turn off image resolution.

True, but we should make sure we have them on in tests. If we do silent skips, we fail to know when this will break. Don't we?

We can't really split the difference. An upgraded cluster isn't required to have these, so extended tests being skipped is irritating but not essential.

soltysh · 2017-06-21T15:28:52Z

And you're missing updated completions.

A lot of times, you want to be able to reuse a bit of config that works fine elsewhere. By adding the `alpha.image.policy.openshift.io/resolve-names` annotation with value `*` to the object you can request that resolution attempt to match all image references to an image stream tag in the current namespace, regardless of their own policy. This makes it easy to transition to looking up image stream tags by name. Extend `oc set image-lookup` to accept `deploy/foo` and to be able to list and display non image stream resources.

smarterclayton · 2017-06-27T00:52:03Z

[test]

…

On Mon, Jun 26, 2017 at 8:07 PM, OpenShift Bot ***@***.***> wrote: continuous-integration/openshift-jenkins/test FAILURE ( https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2646/) (Base Commit: d007fa3 <d007fa3>) (PR Branch Commit: 7ca6090 <7ca6090> ) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#14795 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p75wd_FzarLeuSi3GH5m4yKTUPeMks5sIEfJgaJpZM4OAcZb> .

smarterclayton · 2017-06-27T03:02:19Z

Updates made, tests added.

soltysh

One nit, but this LGTM.

soltysh · 2017-06-27T14:47:51Z

test/cmd/image-lookup.sh

+os::cmd::expect_success_and_text "oc set image-lookup secrets --list" "false"
+
+# Clear resources
+os::cmd::expect_success "oc delete deploy,dc,rs,rc,pods --all"


You're not creating DC, so no need to remove it.

Ah. I may come back and add one in a follow on.

soltysh · 2017-06-27T14:51:40Z

Flake #14043

[merge]

smarterclayton · 2017-06-27T22:52:58Z

[test]

openshift-bot · 2017-06-27T22:55:37Z

Evaluated for origin test up to 7ca6090

openshift-bot · 2017-06-28T00:47:29Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2730/) (Base Commit: a67ac87) (PR Branch Commit: 7ca6090)

soltysh · 2017-06-28T11:16:38Z

Flake #14043

[merge]

openshift-bot · 2017-06-28T11:23:27Z

Evaluated for origin merge up to 7ca6090

openshift-bot · 2017-06-28T13:23:32Z

continuous-integration/openshift-jenkins/merge FAILURE (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/1161/) (Base Commit: 6bbad88) (PR Branch Commit: 7ca6090)

smarterclayton added this to the 3.6.0 milestone Jun 21, 2017

smarterclayton force-pushed the policy_2 branch from ff179d2 to 4cf47e9 Compare June 21, 2017 04:32

soltysh suggested changes Jun 21, 2017

View reviewed changes

smarterclayton force-pushed the policy_2 branch 3 times, most recently from 7ed26b7 to 79f66e9 Compare June 24, 2017 21:56

smarterclayton force-pushed the policy_2 branch from 79f66e9 to 7ca6090 Compare June 26, 2017 21:54

soltysh approved these changes Jun 27, 2017

View reviewed changes

smarterclayton merged commit 20c77cb into openshift:master Jun 28, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow objects to request resolution, not just images #14795

Allow objects to request resolution, not just images #14795

smarterclayton commented Jun 21, 2017 •

edited

Loading

soltysh left a comment

soltysh Jun 21, 2017

soltysh Jun 21, 2017

smarterclayton Jun 21, 2017

soltysh Jun 21, 2017

soltysh Jun 21, 2017

smarterclayton Jun 21, 2017

soltysh Jun 21, 2017

smarterclayton Jun 26, 2017 •

edited

Loading

soltysh commented Jun 21, 2017

smarterclayton commented Jun 27, 2017 via email

smarterclayton commented Jun 27, 2017 via email

soltysh left a comment

soltysh Jun 27, 2017

smarterclayton Jun 27, 2017

soltysh commented Jun 27, 2017

smarterclayton commented Jun 27, 2017

openshift-bot commented Jun 27, 2017

openshift-bot commented Jun 28, 2017

soltysh commented Jun 28, 2017

openshift-bot commented Jun 28, 2017

openshift-bot commented Jun 28, 2017

Allow objects to request resolution, not just images #14795

Allow objects to request resolution, not just images #14795

Conversation

smarterclayton commented Jun 21, 2017 • edited Loading

soltysh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarterclayton Jun 26, 2017 • edited Loading

Choose a reason for hiding this comment

soltysh commented Jun 21, 2017

smarterclayton commented Jun 27, 2017 via email

smarterclayton commented Jun 27, 2017 via email

soltysh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soltysh commented Jun 27, 2017

smarterclayton commented Jun 27, 2017

openshift-bot commented Jun 27, 2017

openshift-bot commented Jun 28, 2017

soltysh commented Jun 28, 2017

openshift-bot commented Jun 28, 2017

openshift-bot commented Jun 28, 2017

smarterclayton commented Jun 21, 2017 •

edited

Loading

smarterclayton Jun 26, 2017 •

edited

Loading