Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix up latest tags and add new scl image versions #5409

Merged
merged 1 commit into from
Oct 29, 2015

Conversation

bparees
Copy link
Contributor

@bparees bparees commented Oct 26, 2015

No description provided.

@bparees
Copy link
Contributor Author

bparees commented Oct 26, 2015

@smarterclayton @soltysh ptal. Note that latest still points to the older versions for now because the SCL images aren't actually released yet, so the rhel images aren't available. So we'll need to update this one more time before 3.1 ships, unless we want to ship 3.1 using the existing platform versions.

@bparees
Copy link
Contributor Author

bparees commented Oct 26, 2015

(also note that i think the reference logic was backwards before... i had the "2.0" tag following/referencing the "latest" tag in the spec, instead of the other way around. that is fixed with this PR.

@smarterclayton
Copy link
Contributor

LGTM

@soltysh
Copy link
Contributor

soltysh commented Oct 27, 2015

I'm removing my LGTM, because with current configuration spec.dockerImageRepository should be removed from all ISs, it's useless right now, that you manually specify each tag.

@@ -14,7 +14,11 @@
"dockerImageRepository": "openshift/ruby-20-centos7",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this and every occurence, where you specify images to be imported manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i was debating whether to leave that or not... if i remove it, nothing (ie tags we don't specify explicitly) will get automatically imported, i'm not sure that's what we want. so i think it's appropriate to leave it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm - that may actually be what we want. We only want to import tags we
whitelist for openshift customers.

On Tue, Oct 27, 2015 at 12:43 PM, Ben Parees notifications@github.com
wrote:

In examples/image-streams/image-streams-centos7.json
#5409 (comment):

@@ -14,7 +14,11 @@
"dockerImageRepository": "openshift/ruby-20-centos7",

yeah i was debating whether to leave that or not... if i remove it,
nothing (ie tags we don't specify explicitly) will get automatically
imported, i'm not sure that's what we want. so i think it's appropriate to
leave it.


Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/5409/files#r43149082.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. if we're in agreement that we only want to import explicit tags i'll remove the dockerImageRepository field.

@soltysh
Copy link
Contributor

soltysh commented Oct 27, 2015

LGTM, thanks @bparees !

@@ -64,7 +64,7 @@ oc describe istag/mysql:latest
[ "$(oc describe istag/mysql:latest | grep "Image Created:")" ]
[ "$(oc describe istag/mysql:latest | grep "Image Name:")" ]
name=$(oc get istag/mysql:latest --template='{{ .image.metadata.name }}')
imagename="isimage/mysql@${name:0:7}"
imagename="isimage/mysql@${name}"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarterclayton i'm not clear on why were were truncating this value previously? but it seems to be causing failures now that we have two valid images that start with mysql

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To verify that you can retrieve images by their short ID. Which we still
want to do, but if there are two we should be able to choose the first.
I'm not sure why there are 2 now though.

On Tue, Oct 27, 2015 at 11:58 PM, Ben Parees notifications@github.com
wrote:

In test/cmd/images.sh
#5409 (comment):

@@ -64,7 +64,7 @@ oc describe istag/mysql:latest
[ "$(oc describe istag/mysql:latest | grep "Image Created:")" ]
[ "$(oc describe istag/mysql:latest | grep "Image Name:")" ]
name=$(oc get istag/mysql:latest --template='{{ .image.metadata.name }}')
-imagename="isimage/mysql@${name:0:7}"
+imagename="isimage/mysql@${name}"

@smarterclayton https://github.com/smarterclayton i'm not clear on why
were were truncating this value previously? but it seems to be causing
failures now that we have two valid images that start with mysql


Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/5409/files#r43216441.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mysql 55 and mysql 56.

Ben Parees | OpenShift
On Oct 28, 2015 12:23 AM, "Clayton Coleman" notifications@github.com
wrote:

In test/cmd/images.sh
#5409 (comment):

@@ -64,7 +64,7 @@ oc describe istag/mysql:latest
[ "$(oc describe istag/mysql:latest | grep "Image Created:")" ]
[ "$(oc describe istag/mysql:latest | grep "Image Name:")" ]
name=$(oc get istag/mysql:latest --template='{{ .image.metadata.name }}')
-imagename="isimage/mysql@${name:0:7}"
+imagename="isimage/mysql@${name}"

To verify that you can retrieve images by their short ID. Which we still
want to do, but if there are two we should be able to choose the first. I'm
not sure why there are 2 now though.
… <#150acafb50312f2c_>
On Tue, Oct 27, 2015 at 11:58 PM, Ben Parees notifications@github.com
wrote: In test/cmd/images.sh <#5409 (comment)
https://github.com/openshift/origin/pull/5409#discussion_r43216441>: >
@@ -64,7 +64,7 @@ oc describe istag/mysql:latest > [ "$(oc describe
istag/mysql:latest | grep "Image Created:")" ] > [ "$(oc describe
istag/mysql:latest | grep "Image Name:")" ] > name=$(oc get
istag/mysql:latest --template='{{ .image.metadata.name }}') >
-imagename="isimage/mysql@${name:0:7}" > +imagename="isimage/mysql@${name}"
@smarterclayton https://github.com/smarterclayton <
https://github.com/smarterclayton> i'm not clear on why were were
truncating this value previously? but it seems to be causing failures now
that we have two valid images that start with mysql — Reply to this email
directly or view it on GitHub <
https://github.com/openshift/origin/pull/5409/files#r43216441>.


Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/5409/files#r43217260.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To verify that you can retrieve images by their short ID. Which we still want to do, but if there are two we should be able to choose the first. I'm not sure why there are 2 now though.

what is the "short id"? what this code was grabbing was something like "mysql@sha:"

and there are now 2 because you have the mysql56 and mysql55 images, both of which start with that string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then grow the length. The sha is different

On Wed, Oct 28, 2015 at 1:11 PM, Ben Parees notifications@github.com
wrote:

In test/cmd/images.sh
#5409 (comment):

@@ -64,7 +64,7 @@ oc describe istag/mysql:latest
[ "$(oc describe istag/mysql:latest | grep "Image Created:")" ]
[ "$(oc describe istag/mysql:latest | grep "Image Name:")" ]
name=$(oc get istag/mysql:latest --template='{{ .image.metadata.name }}')
-imagename="isimage/mysql@${name:0:7}"
+imagename="isimage/mysql@${name}"

To verify that you can retrieve images by their short ID. Which we still
want to do, but if there are two we should be able to choose the first. I'm
not sure why there are 2 now though.

what is the "short id"? what this code was grabbing was something like
"mysql@sha:"

and there are now 2 because you have the mysql56 and mysql55 images, both
of which start with that string.


Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/5409/files#r43286739.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, i'm just trying to understand what "retrieve images by their short ID" means... is short ID specifically defined, or are you just saying "something shorter than the full id"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the REST API uses the provided image id for image stream images as a
prefix and will find all matches - if there is just one, it is accepted, if
there are multiple, it gives an error. Basically like Git.

On Wed, Oct 28, 2015 at 3:06 PM, Ben Parees notifications@github.com
wrote:

In test/cmd/images.sh
#5409 (comment):

@@ -64,7 +64,7 @@ oc describe istag/mysql:latest
[ "$(oc describe istag/mysql:latest | grep "Image Created:")" ]
[ "$(oc describe istag/mysql:latest | grep "Image Name:")" ]
name=$(oc get istag/mysql:latest --template='{{ .image.metadata.name }}')
-imagename="isimage/mysql@${name:0:7}"
+imagename="isimage/mysql@${name}"

Sure, i'm just trying to understand what "retrieve images by their short
ID" means... is short ID specifically defined, or are you just saying
"something shorter than the full id"?


Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/5409/files#r43302985.

@bparees bparees force-pushed the scl_imagestreams branch 3 times, most recently from 085a0de to 8f1f6a9 Compare October 28, 2015 21:42
@bparees
Copy link
Contributor Author

bparees commented Oct 28, 2015

[test]

@bparees
Copy link
Contributor Author

bparees commented Oct 29, 2015

imagestreams appear to be disturbingly unreliable with these changes.... not sure what is going on.

!!! Error in test/cmd/newapp.sh:41
    '[ "$(oc new-app --search mysql | grep -E "Tags:\s+5.5,5.6,latest")" ]' exited with status 1

@smarterclayton
Copy link
Contributor

Individual tags are added in order - it's possible that the end result is
that you're catching flakes on one of the tags. Do a tryuntil oc get
istag/one istag/two

On Oct 28, 2015, at 11:01 PM, Ben Parees notifications@github.com wrote:

imagestreams appear to be disturbingly unreliable with these changes....
not sure what is going on.

!!! Error in test/cmd/newapp.sh:41
'[ "$(oc new-app --search mysql | grep -E
"Tags:\s+5.5,5.6,latest")" ]' exited with status 1


Reply to this email directly or view it on GitHub
#5409 (comment).

@bparees
Copy link
Contributor Author

bparees commented Oct 29, 2015

do they get displayed in sorted order, or did i just get lucky on that in my local testing and i need to make my grep more tolerant?

@smarterclayton
Copy link
Contributor

I thought Michal added a sort in string flexographic order.

On Wed, Oct 28, 2015 at 11:14 PM, Ben Parees notifications@github.com
wrote:

do they get displayed in sorted order, or did i just get lucky on that in
my local testing and i need to make my grep more tolerant?


Reply to this email directly or view it on GitHub
#5409 (comment).

@liggitt
Copy link
Contributor

liggitt commented Oct 29, 2015

flexographic Is my new favorite word

@bparees
Copy link
Contributor Author

bparees commented Oct 29, 2015

@liggitt it's your ability to jump into a pull you weren't tagged in, within mere seconds, that makes me default to the assumption that all comments i read were posted by you. you're everywhere. and yes, +1 to flexographic. I pity my next interview candidate......."how would you write a flexographic sort?"

@smarterclayton
Copy link
Contributor

The look of panic on the interviewee's face would be worth the eternity in Hell.

@bparees bparees force-pushed the scl_imagestreams branch 2 times, most recently from 4396ae3 to 328b98f Compare October 29, 2015 20:42
@bparees
Copy link
Contributor Author

bparees commented Oct 29, 2015

this might finally be passing. [merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6406/) (Image: devenv-rhel7_2594)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to aa60db9

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to aa60db9

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/6406/)

openshift-bot pushed a commit that referenced this pull request Oct 29, 2015
@openshift-bot openshift-bot merged commit f4f3bcd into openshift:master Oct 29, 2015
@bparees bparees deleted the scl_imagestreams branch October 30, 2015 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants