don't create output imagestrem if already exists with newapp

[gabemontero]

Fixes #13169 and https://bugzilla.redhat.com/show_bug.cgi?id=1419801

@openshift/devex ptal

/assign csrwng1 bparees

[openshift-ci-robot]

@gabemontero: GitHub didn't allow me to assign the following users: csrwng1.

Note that only openshift members can be assigned.

In response to this:

Fixes #13169 and https://bugzilla.redhat.com/show_bug.cgi?id=1419801

@openshift/devex ptal

/assign csrwng1 bparees

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[gabemontero]

/assign @csrwng

[gabemontero]

didn't realize or forgot there were new-app integration tests ... several failures there; undoubtedly broke some assumptions there along the way (similar to the cmd test assumptions I had to account for)

so more updates coming

[bparees]

I don't think this will handle the case where a tag needs to be created within an imagestream, e.g. if I run:

$ oc new-app docker.io/ruby:2.0
--> Found Docker image cbfcd91 (19 months old) from docker.io for "docker.io/ruby:2.0"

    * An image stream will be created as "ruby:2.0" that will track this image
    * This image will be deployed in deployment config "ruby"
    * The image does not expose any ports - if you want to load balance or send traffic to this component
      you will need to create a service with 'expose dc/ruby --port=[port]' later
    * WARNING: Image "docker.io/ruby:2.0" runs as the 'root' user which may not be permitted by your cluster administrator

--> Creating resources ...
    imagestream "ruby" created
    deploymentconfig "ruby" created
--> Success
    Run 'oc status' to view your app.

this creates a ruby imagestream with a 2.0 tag (which the DC references)

and then I run:

$ oc new-app docker.io/ruby:2.2

previously i'd get an error that it can't create the ruby imagestream.

w/ these changes i think it'll just skip creating the imagestream entirely, which isn't right either (it'll still create the DC referencing a ruby:2.2 IST, which won't exist).

it needs to add a 2.2 tag to the existing ruby imagestream.

[gabemontero]

OK I think that makes sense, and come to think of it, that crossed my mind during coding, but the distinction between tag and is was not obvious as I started analyzing / changing code and the notion fell by the waste side.

I'll take another stab with this in mind. I certainly have a better understanding of this code now from when I started. So I suspect I'll pick up that particular nuance relatively easy this time.

On the integration test failures I started to look at, the new "don't create the IS" logic did kick in:

I1012 15:34:08.412440 13163 pipeline.go:507] acceptor determined that imagestream centos exists so don't accept

I do think there are at least some cases where these tests were assuming the IS would be in the returned list of obj to create (regardless of the new tag in existing IS distinction). But we'll see for sure as I sort things out.

[gabemontero]

To further clarify, there are certainly tag related fields in ImageRef ... it was their use when I initially searched on them that gave me pause. Researching now ...

[gabemontero]

OK, I have the creation of new imagestreamtag working in case where imagestream already exists

Sorting through the integration tests, and then will push update.

[csrwng]

As mentioned earlier, I'm not sure that we need to pass the image stream that we are not creating to the checkCircularReferences call. If the image stream exists, then checkCircularReferences should be able to find it to determine if we indeed have a circular reference.

[gabemontero]

On Fri, Oct 13, 2017 at 11:26 AM, Cesar Wong ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pkg/generate/app/cmd/newapp.go <#16843 (comment)>: > @@ -849,7 +853,7 @@ func (c *AppConfig) Run() (*AppResult, error) { } } // check for circular reference specifically from the newly generated objects, handling new-app vs. new-build nuances as needed - err = c.checkCircularReferences(objects) + err = c.checkCircularReferences(checkObjects) As mentioned earlier, I'm not sure that we need to pass the image stream that we are not creating to the checkCircularReferences call. If the image stream exists, then checkCircularReferences should be able to find it to determine if we indeed have a circular reference.

Yep, and as I mentioned to @csrwng, `checkCircularReferences` is complaining if we don't pass it in at this point: https://github.com/openshift/origin/blob/master/pkg/generate/app/cmd/newapp.go#L859 When I circle back to this I'll add investigating why that is the case (and why it does not look up / find the existing image stream that is not in the provided list) to the to-do's for this.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16843 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADbadO9yj4Z0-UbE7kN02V8yJpsCBF-Rks5sr4E7gaJpZM4P3czl> .

[gabemontero]

OK, short term, I've pushed updates to address @bparees 's need to create IST's in existing IS's point, and the associated integration test changes because of the new behavior wrt IS's.

I'm not going to dive into the check circular dependency item that @csrwng and I commented on earlier today.

I'll report back when I have findings of relevance.

[gabemontero]

OK, so I've got some details on the difference in circular dependency check behavior when passing in the image stream ... what do do about it is a different question :-)

1. so with the old behavior, where new-app attempts to create an IS that already exists, so when it is used in resolving images:

		// The imagestream is usually being created alongside the buildconfig
		// when new-build is being used, so scan objects being created for it.
		for _, check := range objects {
			if is2, ok := check.(*imageapi.ImageStream); ok {
				if is2.Name == isName {
					isContext = is2
					break
				}
			}
		}

the new IS created is essentially empty, so we end up in the "don't bother anymore block of code", and give up trying to detect circular dependencies:

	if target == nil {
		glog.V(0).Infof("GGM followRefToDockerImage returning %#v", isContext)
		if isContext.Spec.DockerImageRepository == "" {
			// Otherwise, this appears to be a new IS, created by new-app, with very little information
			// populated. We cannot resolve a DockerImage.
			return nil, nil
		}

2. now, with the new behavior, if we do NOT pass in the newly created, empty image stream, the code in fact now reads in the existing IS from master:

		if isContext == nil {
			glog.V(0).Infof("GGM no is so calling getter for is")
			var err error
			isContext, err = c.ImageClient.ImageStreams(isNS).Get(isName, metav1.GetOptions{})
			if err != nil {
				return nil, fmt.Errorf("Unable to check for circular build input/outputs: %v", err)
			}
			glog.V(0).Infof("GGM got from getter is %#v", isContext)
		}

It will find a docker reference and attempt the circular dependency check.

So, now the problem. One of the test cases in test/cmd/newapp.sh is oc new-build --binary php --env X=Y,Z=W -o yaml, which it currently expects to succeed.

HOWEVER, when running, this code determines that the input image is imagestreamtag php:7.0, and the output image is imagestreamtag php:latest.

When we start de-referencing them, that logic uncovers that both of those point to the actual docker image centos/php-70-centos7:latest ... literally centos/php-70-centos7@sha256:bbfaf4670f2eb62...

Hence it throws the circular reference error.

So at first blush,
a) either the test case is incorrect, and if you expect success, you must add --to to get rid of the circular ref
b) or the input and output ISTs generated for oc new-build --binary php --env X=Y,Z=W -o yaml are incorrect, and there is a separate bug which this change has uncovered that we need to chase down

Thoughts (I'm hoping it is a) :-) )?

[bparees]

it seems like something is overlooking namespaces

you've got an imagestreamtag openshift/php:7.0 that points to dockerimage1

and then you've presumably created (on a previous new-app run) an imagestream myproject/php:latest which, at least until you you run a build, should not have any dockerimage references in it.

So

1. that should not be a circular reference (there's no relationship between them).
2. it kinda sounds like the lookup went against openshift/php:latest instead

Now, supposing you did run new-app inside the openshift namespace, where you've got both a openshift/php:7.0 and openshift/php:latest IST, both of which point to dockerimage1:

Creating a buildconfig that outputs to openshift/php:latest may or may not be a circular reference error, i'm not sure. php:latest tracks php:7.0, when you push to php:latest, I am not sure if php:7.0 gets updated or not. If it does, then yes, we need to treat it as a circular reference error. If it does not (because the newly pushed image only shows up in the php:latest IST) then it's not a circular refernce error since it won't trigger the buildconfig's imagechangetrigger (assuming the ICT is for php:7.0).

hth.

[gabemontero]

Yeah doing this out newapp.sh a whole bunch of operations occur before this one runs. I’ll revisit with an eye to namespaces and report back. Thanks

…

On Fri, Oct 13, 2017 at 6:34 PM Ben Parees ***@***.***> wrote: it seems like something is overlooking namespaces you've got an imagestreamtag openshift/php:7.0 that points to dockerimage1 and then you've presumably created (on a previous new-app run) an imagestream myproject/php:latest which, at least until you you run a build, should not have any dockerimage references in it. So 1. that should not be a circular reference (there's no relationship between them). 2. it kinda sounds like the lookup went against openshift/php:latest instead Now, supposing you *did* run new-app inside the openshift namespace, where you've got both a openshift/php:7.0 and openshift/php:latest IST, both of which point to dockerimage1: Creating a buildconfig that outputs to openshift/php:latest may or may not be a circular reference error, i'm not sure. php:latest tracks php:7.0, when you push to php:latest, I am not sure if php:7.0 gets updated or not. If it does, then yes, we need to treat it as a circular reference error. If it does not (because the newly pushed image only shows up in the php:latest IST) then it's not a circular refernce error since it won't trigger the buildconfig's imagechangetrigger (assuming the ICT is for php:7.0). hth. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16843 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADbadK_Gm1JoWhkrK5hxcFKapI2Gy9cuks5sr-VbgaJpZM4P3czl> .

[gabemontero]

OK, so running oc get is -n openshif in newapp.sh just prior to the invocation in question produced No resources found.

Conversely, running oc get is returned what we typically see in the openshift namespace, where the namespace we are currently in is cmd-newapp:

NAME         DOCKER REPO                                TAGS                           UPDATED
cakephp-ex   172.30.169.48:5000/cmd-newapp/cakephp-ex                                  
httpd        172.30.169.48:5000/cmd-newapp/httpd        2.4,latest                     About a minute ago
jenkins      172.30.169.48:5000/cmd-newapp/jenkins      latest,1,2                     About a minute ago
mariadb      172.30.169.48:5000/cmd-newapp/mariadb      latest,10.1                    About a minute ago
mongodb      172.30.169.48:5000/cmd-newapp/mongodb      latest,3.2,2.6 + 1 more...     About a minute ago
mysql        172.30.169.48:5000/cmd-newapp/mysql        latest,5.7,5.6 + 1 more...     About a minute ago
nodejs       172.30.169.48:5000/cmd-newapp/nodejs       latest,0.10,4 + 1 more...      About a minute ago
perl         172.30.169.48:5000/cmd-newapp/perl         latest,5.24,5.20 + 1 more...   About a minute ago
php          172.30.169.48:5000/cmd-newapp/php          latest,7.0,5.6 + 1 more...     About a minute ago
postgresql   172.30.169.48:5000/cmd-newapp/postgresql   9.2,latest,9.5 + 1 more...     About a minute ago
python       172.30.169.48:5000/cmd-newapp/python       2.7,latest,3.5 + 2 more...     About a minute ago
redis        172.30.169.48:5000/cmd-newapp/redis        latest,3.2                     About a minute ago
ruby         172.30.169.48:5000/cmd-newapp/ruby         latest,2.4,2.3 + 2 more...     About a minute ago
wildfly      172.30.169.48:5000/cmd-newapp/wildfly      8.1,latest,10.1 + 2 more...    About a minute ago

With only a single namespace involved then, and per my prior debug, given that both 7.0 and latest tags resolve to the same actualy docker image (i.e. the same sha) that falls into how the code currently determine circular references ... it compares the docker images the tags point to ...

only @bparees 's last paragraph in #16843 (comment) gives me some pause.... will the push to latest, does 7.0 get updated?

so I'm attempting to determine that now .... will post update after lunch :-)

[gabemontero]

Based on my analysis of pkg/image/apis/image/helper.go the answer to questions "does php:7.0 get updated when php:latest tracks php:7.0 and you push to php:latest" appears to be no.

It only works in the other direction ... if you push to php:7.0, php:latest gets updated.

So I am concluding that our circular reference detection logic needs some tweaking ... I will get started on that, as well as revert the cretaeObject/checkObjects change, and then see what the carnage is with the unit / cmd / integration / extended tests are

[gabemontero]

Well ... continuing to read all the various godoc and field references, I'm no so sure ... let's walk through some stuff:

1. checkCircularReferences claims to ensure that no builds that can trigger themselves due to an imagechangetrigger that matches the output destination of the image

2. a build ICT is a is a reference to an ImageStreamTag that will trigger a build when updated

3. Per https://github.com/openshift/origin/blob/master/pkg/image/apis/image/helper.go#L649-L655 on the image stream side if stream.spec.tags[latest].from.name = 2.0, whenever an image is pushed to this stream with the tag 2.0, status.tags[latest].items[0] will also be updated to point at the same image that was just pushed for 2.0.

4. in our example here, php:latest has a from.name to php:7.0

    from:
      kind: ImageStreamTag
      name: "7.0"
    generation: 1
    importPolicy: {}
    name: latest

5. so a change to php:70 will lead to an update of php:latest anyway, so if an ICT is added to the BC, we'll be triggering a build to produce an image that was already updated, no?

Don't we want to prevent both 1) through 5) then as well as does php:7.0 get updated when php:latest tracks php:7.0 and you push to php:latest ?

My head officially hurts :-(

I'm also curious if https://github.com/openshift/origin/blame/master/pkg/image/apis/image/helper.go#L383 was leveraged in the past ...

While waiting on some feedback to the above musings, I'm going to experiment with https://github.com/openshift/origin/blame/master/pkg/image/apis/image/helper.go#L383 along with a couple of other things ...

[bparees]

if a build can trigger itself we should be trying to flag it.

So regardless of the direction (updating latest causes and update in 7.0, or updating 7.0 causes an update in latest), it is potentially a circular reference error. (whether it is or not depends on which of those two is the ICT image and which is the output image. Assuming that it's not symmetrically true, ie that updating either one does not always update the other, regardless of the direction of the reference)

Anyway this should be pretty easy to test... create two tags, on referencing the other, push to one, see if the other gets updated (check the image history of the tag). then push to the other, and do the same.

[gabemontero]

OK, good, that is were I was headed ... (regardless of direction, if a build can trigger itself, even if it is not symmetrically true, it is potentially a circular ref, etc.)

Yeah, aside from trying it out, the image code seems pretty straight forward on its handling of the from ref within the tag.

So I'm going to move forward with correcting some tests that were getting by with potential circular refs.

[gabemontero]

OK, I've pushed an update that

1. no longer distinguishes between created and checkable objects
2. still will create new ISTs in existing image streams
3. fix the cmd tests where circular ref errors were now possible because we are reusing existing image streams per our recent discussion above
4. turns out, no integration test changes were needed ... there appears to be either a bug or unexplained odd behavior with getters and the integration tests ... I'll touch base with some folks in the office on Wednesday ... I left the short term workaround with some comments on what happened to help explain what was going on

[bparees]

shouldn't this return?

also typo in unanticipated.

[gabemontero]

yeah I waffled on this a bit - original thought was to try the get regardless ... but yeah, return true just like image.IsKindOrLegacy("ImageStream", gk) makes sense ... part of next push

[bparees]

imgstrm.Name==is.Name check seems redundant, did you find it necessary?

[gabemontero]

This is the piece regarding the odd behavior around getting image streams in the integration tests I referred to in my recent comment ... yes, this should not be necessary, but instead of nil I was getting this empty imagestream back ... I plan on asking the appropriate folks about this and will report back

[bparees]

should return? and typo in unanticipated.

[bparees]

why did you have to add the --to=binary?

I would have expected that this command would pick up the input "php" image from the openshift namespace, and create a target output "php" in the current namespace, which should not be a circular reference.

[gabemontero]

this centers around the whole circular dependency discussion we've been having .... the result is that with the php images present in the test's namespace (not the openshift namespace) is that php:latest has a from link to php:7.0, and with them pointing to the same image, the current logic that compares the docker image ref fails

when this test runs, there ARE NO IMAGESTREAMS in the openshift namespace

the prior incarnation did not fail because we were not reusing the real image stream, but doing the circular check against our newly created image stream that was empty

[bparees]

ok, if there is an existing php imagestream (with a 7.0 and latest tag as you describe, and the update behavior you confirmed) then yeah, it should have been caught as a circular reference before and i agree w/ adding the --to here.

that said, i'm not seeing that behavior... i created a php tag with a 7.0 and a latest that follows 7.0, then i did a build that pushed to latest and it did not update the 7.0 tag:

latest tag after push to latest tag:

- items:
    - created: 2017-10-17T14:24:23Z
      dockerImageReference: 172.30.66.174:5000/p3/php@sha256:0d3de95379d5fb1192e8210886fe8b6bc082c3693da3504bd39b2bc51bc17b85
      generation: 2
      image: sha256:0d3de95379d5fb1192e8210886fe8b6bc082c3693da3504bd39b2bc51bc17b85
    - created: 2017-10-17T14:22:19Z
      dockerImageReference: centos/php-70-centos7@sha256:e00e498810997b855ae23f1531d2e2df490beab37c790a5a50587df4bfcb15e1
      generation: 2
      image: sha256:e00e498810997b855ae23f1531d2e2df490beab37c790a5a50587df4bfcb15e1
    tag: latest

7.0 tag after push to latest tag:

    tag: "7.0"
  - items:
    - created: 2017-10-17T14:22:19Z
      dockerImageReference: centos/php-56-centos7@sha256:7c303efa89bc3513e0da6028b86a8954ab5836706988bc257ffe0d16ae2374bc
      generation: 2
      image: sha256:7c303efa89bc3513e0da6028b86a8954ab5836706988bc257ffe0d16ae2374bc

[bparees]

(and i don't think the latest tag will get updated on a push to 7.0 either. I think you'd have to push to 7.0 and then explicitly do an import-image)

[bparees]

same here, i'm not convinced this should be being reported as a circular reference error, would like to understand why it is(was).

[gabemontero]

see above ... or if need be, let's discuss in scrum

[bparees]

why was this needed? and why the --ignore-not-found? Our tests should be deterministic so either we should expect this is there to be deleted, or not.

[gabemontero]

Leftover from prior incarnations ... I'll remove ... probably should be fine ...if things break, I'll be able to provide a specific reason why

[gabemontero]

tests pass with this removed .... will be part of next push

[bparees]

we should know at this point in the test if these deletes are necessary or not.

[gabemontero]

i'll apply the same remove / diagnose if something is amiss methodology

[gabemontero]

tests pass with this removed .... will be part of next push

[bparees]

with? without?

[bparees]

also should probably log any error.

[gabemontero]

yeah the comment was a leftover from a prior implementation ... removing

adding error log as well ... part of next push

[gabemontero]

sorry ... actually, now I thinks it makes sense to return an error like we do elsewhere in this method ... doing that (unless tests fail in unexpected ways :-) )

[gabemontero]

OK, to recap our scrum and post-scrum IRC pow-wow:

1. confirmed/everyone on the same page that if php:latest refs php:7.0 and you push to 7.0, latest is updated (which as I noted earlier is what I saw in the code); with the same structure, also as expected, if you push to latest, 7.0 is not updated

2. We settled that the source image ICT trigger scenario I was getting tripped up on was not a scenario we wanted to flag on an error (i.e. my interpretation of the godoc was not what we wanted) ... will look at tweaking godoc as part of this work

3. that means that we don't flag src image sha == output image sha as a problem ... so reworking circ ref check ... going to move toward the helper method in image api

4. with associated tweaks in cmd test to follow

[gabemontero]

OK .... modification after more @csrwng @bparees exchanges:

1. still ultimately do the input docker image ref compare with output docker image ref
2. but in doing so, if the output image is an IST with a ref to another IST, stop ... don't use that docker image ref ... this allows for oc new-build --binary php to not require --to
3. but it does allow for circular ref test cases without an IST from another IST link like https://github.com/openshift/origin/blob/master/test/integration/newapp_test.go#L1564-L1591 to still work as expected ... meaning an error is logged
4. however, for the newly creating a needed IST when reusing imagestreams aspect of this PR, we need to make sure that if the output IST is non-standard/not latest ... i.e. the passed in --to ... that we make sure the input/output tags are not the same ... i.e. a docker file had FROM centos:baz, but the user also supplied --to=baz

I'm still going to also add some infinite recursive loop detection, and per some exchanges with David Eads (I'll avoid tagging him to spare him this PR's chatter), most likely the integration test client is doing something with mocks that is getting us the bogus image stream getter behavior ... will track that down as well.

whew

[gabemontero]

OK I've added the multi-imagestream circular check and associated test, and pushed that to the PR.

I'm still hashing over the legacy tests that involve the CircularOutputError via the traversal down to the docker image ref and comparisons if the images are the same, and moving instead to @bparees 's 1) and 2) from the last set of comments.

[bparees]

is this not mandatory for a tag ref of kind ImageStreamTag ?

[bparees]

i guess based on the logic below if it doesn't contain a colon it's assumed that From.Name refers to another tag within this imagestream. ok.

[gabemontero]

yep that is what it is

[bparees]

the new circular tag detection check lgtm.

[gabemontero]

OK I took an exploration of not following tags down the docker image ref. If we no longer do that, then supported scenarios like this have changed:

1. the build config's input is an IST, with a from/reference to a given docker image
2. the build config's output is a docker image ref, to the same docker image as the input IST from/ref
3. the integration tests are looking for things to succeed, but with a warning WARNING: output image of \"centos/ruby-22-centos7:latest\" should be different than input

There are 4 or 5 other permutations along those lines.

I'm inclined to continue with the docker image ref resolution. Thoughts?

[gabemontero]

ok I've pushed the rest of the updates for things previously discussed @bparees ... I believe we had settled on being OK with leaving the docker image ref resolution intact, but certainly per #16843 (comment) we can discuss further as needed

[bparees]

lgtm (will continue to hold for 3.8).

regarding checking docker image refs, part of my thinking in suggesting not doing it is that despite what the integration test does, in reality new-app never creates outputs that aren't ISTs. And of course if your input isn't an IST, you can't have an image change trigger.

So in practice today, in all cases that matter you'll always have an input IST and an output IST, and thus should always be able to just check whether the ISTs are circular without ever looking at docker image values.

I think the only ways you get outputs that aren't ISTs are:

1. you are new-app'ing a template (in which case i'd argue it may not be our job to protect you)
2. you're new-app'ing an app.json which is a docker compose format I think that i'm pretty sure we've already deprecated support for.

[gabemontero]

@bparees - I realized during PTO that the new circular check logic should both leverage the client and the list of object to be created.

In adding that aspect today and whole new layer of opportunity for overall improvement of the change arose :-)

I've isolated the new change in a separate commit for ease of review. Of course I can squash once the upcoming round of review is completed.

[bparees]

i don't understand the need for this copy. Assigning stream=blah does not affect the value pointed to by stream. (assigning *stream=blah would, of course).

[gabemontero]

right - over thought it - will adjust

[gabemontero]

update pushed

[bparees]

@gabemontero squash and lgtm.
/hold cancel

[gabemontero]

squash pushed @bparees

[bparees]

/lgtm

[openshift-merge-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, gabemontero

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

• pkg/generate/OWNERS [bparees]
• test/cmd/OWNERS [bparees]
• test/integration/OWNERS [bparees]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-bot]

/retest

Please review the full test history for this PR and help us cut down flakes.

[openshift-merge-robot]

Automatic merge from submit-queue.

[openshift-ci-robot]

@gabemontero: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/openshift-jenkins/experimental/unit	b33ed12	link	/test origin-ut
ci/openshift-jenkins/extended_conformance_install_update	697ee8e	link	/test extended_conformance_install_update
ci/openshift-jenkins/extended_conformance_install	697ee8e	link	/test extended_conformance_install

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.