fix origin openshift-ansible deploys on centos #2772
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2772 +/- ##
=========================================
- Coverage 46.95% 46.8% -0.15%
=========================================
Files 86 86
Lines 12781 12774 -7
=========================================
- Hits 6001 5979 -22
- Misses 6226 6250 +24
+ Partials 554 545 -9
Continue to review full report at Codecov.
|
| ANSIBLE_DEPLOY_TYPE="origin" | ||
| IMAGE_TYPE="${SERVICE_TYPE}" | ||
| IMAGE_PREFIX="openshift" | ||
| # FIXME: These versions are set to deal with differences in how Origin and OCP |
There was a problem hiding this comment.
Topic for discussion in the next arch call @pweil- @jim-minter ?
There was a problem hiding this comment.
I was discussing this on IRC with maxamillion and didn't update the PR - I'm of the opinion that this can/should be removed from the PR before it merges.
There was a problem hiding this comment.
(at least all the new PROMETHEUS_VERSION bits - the existing ANSIBLE_DEPLOY_TYPE/IMAGE_PREFIX stuff is required and will end up getting baked into the image one day)
There was a problem hiding this comment.
What specifically should be removed?
| @@ -74,6 +72,10 @@ localmaster: | |||
| template_service_broker_image_name: "IMAGE_TYPE-template-service-broker" | |||
| template_service_broker_version: "vVERSION" | |||
|
|
|||
| # NOTE: This is redundant, but required to handle version skew between | |||
| # openshift-ansible in Origin and OCP | |||
| openshift_examples_content_version: "vSHORT_VER" | |||
There was a problem hiding this comment.
is SHORT_VER still defined in this PR? Looks like it's been dropped?
There was a problem hiding this comment.
it's not defined explicitly because I can do a bash parameter expansion to obtain the value and the sed statement is still there with s|SHORT_VER|${VERSION%.*}|g;
There was a problem hiding this comment.
thanks, was looking in the wrong place
| @@ -62,6 +71,15 @@ set -x | |||
| routerLBHost="{{.RouterLBHostname}}" | |||
| routerLBIP=$(dig +short $routerLBHost) | |||
|
|
|||
| # NOTE: The version of openshift-ansible for origin defaults the ansible var | |||
| # openshift_prometheus_node_exporter_image_version correctly as needed by | |||
| # origin, but for OCP it does not. | |||
There was a problem hiding this comment.
This is fixed in openshift/openshift-ansible@c27a0f4, which is in >= 3.9.15. Please add a note to that effect, and a TODO to remove this once we're installing >= 3.9.15.
|
needs rebase + confirm that it's tested, otherwise looks good |
Signed-off-by: Adam Miller <[email protected]>
|
rebased and squashed, confirm deploy of both OCP and Origin v3.9.x |
|
lgtm, @CecileRobertMichon please merge |
|
@CecileRobertMichon looking at the CircleCI output, I don't think the failure is related to my change. Any chance we can restart the checks? |
Signed-off-by: Adam Miller [email protected]
What this PR does / why we need it:
This fixes a number of issues with the OpenShift Origin openshift-ansible deploy on CentOS images.