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

add clarification around oc version and image tags #486

Merged
merged 1 commit into from
Jan 22, 2018
Merged

add clarification around oc version and image tags #486

merged 1 commit into from
Jan 22, 2018

Conversation

gabemontero
Copy link
Contributor

Fixes #477

@openshift/sig-developer-experience ptal

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 19, 2018
README.md Outdated
---------------------------------

To assist in interacting with the OpenShift API server while using this image, the `oc` binary, the CLI command for OpenShift, has been
installed in this image.
Copy link
Contributor

Choose a reason for hiding this comment

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

installed in the master and slave images defined in this repository.

README.md Outdated
To assist in interacting with the OpenShift API server while using this image, the `oc` binary, the CLI command for OpenShift, has been
installed in this image.

However, it needs to noted that backward compatibility is not guaranteed between different versions of `oc` and the OpenShift
Copy link
Contributor

Choose a reason for hiding this comment

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

s/to noted/to be noted/

README.md Outdated
policy used is warranted to guarantee consistency around what image is being used on each of your nodes.

**Notice: the `latest` tag for the RHEL7 images will point to 3.6 for the time being in order to support users whose pull policies for
their Kubernetes Plugin configuration lack the "pull always" setting. This way, they will have an older `oc` client which should be able to
Copy link
Contributor

Choose a reason for hiding this comment

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

it's unrelated to pullalways. The rhel7 images point to the 3.6 tag (not just for the time being but probably forever) to ensure users on older clusters and who have older slave configurations that point to ":latest" will continue to work (will continue to pick up the v3.6 oc binary).

New slave configurations use version specific tags (as well as pullalways) to ensure the right image is used for their cluster.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to be as non-committal as possible wrt "time being" vs. "probably forever"; just seemed rash to say "forever"... perhaps just remove the "time being" phrase and not substitute any other indication of time frame ?

otherwise sure I'll remove the pull always piece, just reference "to ensure users on older clusters and who have older slave configurations that point to ":latest" will continue to work (will continue to pick up the v3.6 oc binary)."

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe "indefinitely" or "for the foreseeable future" instead. "For the time being" implies to me that we have plans to change it at some point when we get to it, which we don't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll go with indefinitely

README.md Outdated
| :-------------------------- | ------------------------ |
| `jenkins-*-centos7:v3.7` | 3.7 `oc` binary |
| `jenkins-*-centos7:v3.6` | 3.6 `oc` binary |
| `jenkins-*-centos7:latest` | 3.9 `oc` binary |
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that if you state 3.9 here, it'll need to be changed next release. Might be better to say "whatever the latest version is" (or words to that effect)?

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 ... it is literally "the version of oc present in the docker.io/openshift/origin:latest image at the time the jenkins images with the latest tag were built".

I'll work on some consumable form of that. Suggestions are of course welcome.

README.md Outdated
| `jenkins-*-centos7:latest` | 3.9 `oc` binary |
| `jenkins-*-rhel7:v3.7` | 3.7 `oc` binary |
| `jenkins-*-rhel7:v3.6` | 3.6 `oc` binary |
| `jenkins-*-rhel7:latest` | 3.6 `oc` binary |
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth reversing the two paragraphs that follow and putting an asterisk here to link the following paragraph more closely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I imagine there is some trick with markdown language to employ the asterisk like you are suggesting .... unless you know different, I'll start hunting for examples / doc references

@gabemontero
Copy link
Contributor Author

response to comments pushed in separate commits

README.md Outdated

**Notice: There is an additional consideration with the pod configurations for the Kubernetes Plugin; earlier versions of this image
did not specify the "pull always" policy for the default agents/slaves configured. As a result, users may have older/different images on
your nodes depending when the images were pulled. Staring with the 3.7 release, the default changed to "pull always" to avoid this problem
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Staring/Starting/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

update pushed

@bparees
Copy link
Contributor

bparees commented Jan 22, 2018

one last nit and lgtm

@gabemontero
Copy link
Contributor Author

thanks @bparees

I'll give @jim-minter some time before squashing and merging

@jim-minter
Copy link
Contributor

lgtm

@gabemontero
Copy link
Contributor Author

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for jenkins merge up to ff4523e

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 22, 2018

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_jenkins_images/94/) (Base Commit: 40b6428) (PR Branch Commit: ff4523e) (Image: devenv-rhel7_95)

@openshift-bot openshift-bot merged commit 6d8a798 into openshift:master Jan 22, 2018
@gabemontero gabemontero deleted the clarify-oc-version branch January 23, 2018 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants