Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Fix oreg_url to work for our CI tests #3480

Merged
merged 1 commit into from
Jul 17, 2018
Merged

Fix oreg_url to work for our CI tests #3480

merged 1 commit into from
Jul 17, 2018

Conversation

0xmichalis
Copy link
Contributor

@0xmichalis 0xmichalis commented Jul 16, 2018

@mjudeikis
Copy link
Contributor

/lgtm

@0xmichalis
Copy link
Contributor Author

Unfortunately the bot is dead, @CecileRobertMichon @jackfrancis mind merging this?

@0xmichalis
Copy link
Contributor Author

Unfortunately the bot is dead, @CecileRobertMichon @jackfrancis mind merging this?

Hold on, I think there is something more missing.

@0xmichalis
Copy link
Contributor Author

This should be ready now but it's unclear why go-bindata does not cooperate. I have updated my local binary and running make validate-generated locally passes for me...

@jim-minter
Copy link
Member

@Kargakis you have a different version of go-bindata to the CI system?

@@ -47,7 +47,7 @@ sed -i -e "s#--loglevel=2#--loglevel=4#" /etc/sysconfig/${SERVICE_TYPE}-master-c

rm -rf /etc/etcd/* /etc/origin/master/* /etc/origin/node/*

MASTER_OREG_URL="$IMAGE_PREFIX/$IMAGE_TYPE"
MASTER_OREG_URL="$IMAGE_PREFIX/$IMAGE_TYPE-${component}:${version}"
Copy link
Member

Choose a reason for hiding this comment

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

@Kargakis is it correct that ${component}:${version} is not shell /single/ quoted? Can this work as is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dollar signs need to be escaped, fixed!

@0xmichalis
Copy link
Contributor Author

@Kargakis you have a different version of go-bindata to the CI system?

Both CircleCI and I are fetching go-bindata on the spot (go get github.com/go-bindata/go-bindata/...)

@CecileRobertMichon
Copy link
Contributor

@Kargakis let me know when this is ready, I'll run the tests and merge it

@0xmichalis
Copy link
Contributor Author

this is ready now

@codecov
Copy link

codecov bot commented Jul 17, 2018

Codecov Report

Merging #3480 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3480      +/-   ##
==========================================
- Coverage   55.95%   55.94%   -0.01%     
==========================================
  Files         105      105              
  Lines       15919    15888      -31     
==========================================
- Hits         8907     8889      -18     
+ Misses       6262     6253       -9     
+ Partials      750      746       -4

@0xmichalis
Copy link
Contributor Author

For some reason, with this fix ansible breaks the image config format for the router and the registry

failed: [localhost] (item={u'name': u'router', u'certificate': {u'keyfile': u'/etc/origin//master/openshift-router.key', u'certfile': u'/etc/origin//master/openshift-router.crt', u'cafile': u'/etc/origin//master/ca.crt'}, u'replicas': u'1', u'serviceaccount': u'router', u'namespace': u'default', u'selector': u'region=infra', u'edits': [{u'action': u'put', u'key': u'spec.strategy.rollingParams.intervalSeconds', u'value': 1}, {u'action': u'put', u'key': u'spec.strategy.rollingParams.updatePeriodSeconds', u'value': 1}, {u'action': u'put', u'key': u'spec.strategy.activeDeadlineSeconds', u'value': 21600}], u'images': u'openshift/origin-${component}:${version}-${component}:${version}', u'stats_port': 1936, u'ports': [u'80:80', u'443:443']}) => {"changed": false, "item": {"certificate": {"cafile": "/etc/origin//master/ca.crt", "certfile": "/etc/origin//master/openshift-router.crt", "keyfile": "/etc/origin//master/openshift-router.key"}, "edits": [{"action": "put", "key": "spec.strategy.rollingParams.intervalSeconds", "value": 1}, {"action": "put", "key": "spec.strategy.rollingParams.updatePeriodSeconds", "value": 1}, {"action": "put", "key": "spec.strategy.activeDeadlineSeconds", "value": 21600}], "images": "openshift/origin-${component}:${version}-${component}:${version}", "name": "router", "namespace": "default", "ports": ["80:80", "443:443"], "replicas": "1", "selector": "region=infra", "serviceaccount": "router", "stats_port": 1936}, "msg": {"cmd": "/usr/bin/oc create -f /tmp/router-9uW9yF -n default", "results": {}, "returncode": 1, "stderr": "Error from server: error when creating \"/tmp/router-9uW9yF\": timeout\n", "stdout": ""}}

Error is unrelated but you can see that the images field is wrong (openshift/origin-${component}:${version}-${component}:${version}).

@0xmichalis
Copy link
Contributor Author

For some reason, with this fix ansible breaks the image config format for the router and the registry

OK, I think I figured this out (once again).

@jim-minter
Copy link
Member

lgtm - @CecileRobertMichon or @jackfrancis please merge

@CecileRobertMichon CecileRobertMichon merged commit f668eb7 into Azure:master Jul 17, 2018
@ghost ghost removed the in progress label Jul 17, 2018
@0xmichalis 0xmichalis deleted the oreg-url-fix branch July 17, 2018 21:43
kkmsft pushed a commit to kkmsft/acs-engine that referenced this pull request Jul 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants