make openshift start --write-config take a dir#1737
make openshift start --write-config take a dir#1737openshift-bot merged 1 commit intoopenshift:masterfrom
Conversation
deads2k
force-pushed
the
deads-openshift-start-take-dir
branch
from
2e61232 to
2ccd6fa
Compare
|
[test] |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1912/) |
|
@deads2k thanks. concept/intention looks good to me. |
hack/test-cmd.sh
Outdated
There was a problem hiding this comment.
why do this in addition to create-master-certs / create-node-config?
There was a problem hiding this comment.
Creates the master config pointing to certs in a non-default location.
There was a problem hiding this comment.
so are we expecting all the certs to exist already? if so, should we do --create-certs=false so we know if create-master-certs and create-node-config didn't make enough things to satisfy openshift start? same comment in other test scripts that use this setup pattern, I guess
There was a problem hiding this comment.
When running with a config file, certs are never created.
There was a problem hiding this comment.
we're not running from config here, we're writing it... wouldn't that generate the certs if missing?
There was a problem hiding this comment.
ah, just got down to where you removed certargs
There was a problem hiding this comment.
nevermind... --create-certs is just a raw boolean now... think we should pass --create-certs=false here?
There was a problem hiding this comment.
Refresh your diff. Got that a couple commits ago.
deads2k
force-pushed
the
deads-openshift-start-take-dir
branch
from
2ccd6fa to
194ee2c
Compare
|
comments so far addressed. |
hack/test-cmd.sh
Outdated
There was a problem hiding this comment.
NODE_CONFIG_DIR already includes node-${KUBELET_HOST}
There was a problem hiding this comment.
looking to see why this didn't break the test.
There was a problem hiding this comment.
ah, consistently wrong.
|
comments so far addressed. |
There was a problem hiding this comment.
To me, @liggitt comment here makes sense, any reason why its not called?
|
Other things I found:
|
|
that's all the comments I have... once those are done and tests pass, squash, and coordinate with @sdodson on merge timing. should also probably email the devlist once it's in the merge queue |
deads2k
force-pushed
the
deads-openshift-start-take-dir
branch
from
781c598 to
79bb37e
Compare
|
Also queue up a doc PR to fix up http://docs.openshift.org/latest/dev_guide/master_node_configuration.html |
|
(and probably other places in doc that reference openshift.local.certificates, etc) |
deads2k
force-pushed
the
deads-openshift-start-take-dir
branch
from
79bb37e to
7e953e3
Compare
deads2k
force-pushed
the
deads-openshift-start-take-dir
branch
from
7e953e3 to
b2b69cc
Compare
|
@derekwaynecarr care to do a final check? |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1697/) (Image: devenv-fedora_1376) |
deads2k
force-pushed
the
deads-openshift-start-take-dir
branch
from
b2b69cc to
40be29b
Compare
|
bad rebase in dns test. re[merge] |
|
Evaluated for origin up to 40be29b |
Merged by openshift-bot
5 participants
This changes the
--write-configparameter to take a directory (instead of boolean). It eliminates--cert-dirand always creates the certs directly in the config directory.Since it is a single directory, the master config dir can be bundled up as a secret for use in a pod.
@liggitt review
@sdodson fyi