*: remove etcd nodes #168
*: remove etcd nodes #168
Conversation
openshift-ci-robot
added
size/L
|
/ok to test |
|
Is the first commit still wip? |
|
@yifan-gu It was. Your timing is impeccable. |
openshift-bot
added
needs-rebase
|
This can merge once etcd is removed from the release branch. |
openshift-bot
added
the
needs-rebase
Now that the etcd members have been moved to the master nodes, there is no need for the etcd nodes.
This should have been remove back in 45fa0e4.
openshift-bot
removed
the
needs-rebase
|
The e2e-aws error was: so we're still missing something. Maybe we do need to re-land openshift/release#1274, despite my earlier comments about that being optional? Or maybe there's a way to load the roles with enough structure that YAML entries for roles we don't support are ignored. |
Until we can re-land [1] (which is currently reverted by [2]). With the etcd node pool still defined in the release template, we're getting [3]: time="2018-08-27T18:50:53Z" level=fatal msg="unrecognized role: etcd" With this commit, we ignore that entry (just as Go's JSON and YAML unmarshallers ignore other properties which aren't reflected in the target Go structures). A better long-term fix would probably be adding more structure to node pools, but I'm punting on that for now. [1]: openshift/release#1274 [2]: openshift/release#1284 [3]: https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_installer/168/pull-ci-origin-installer-e2e-aws/710/build-log.txt
| } | ||
|
|
||
| func parseIgnFile(filePath string) (*ignconfigtypes.Config, error) { | ||
| func parseIgnFile(filePath string) (ignconfigtypes.Config, error) { |
There was a problem hiding this comment.
I'd rather keep the pointer here to avoiding needless copying. But that's a minor nit compared to this PR as a whole, so I'm fine punting a return to pointers to follow-up work.
There was a problem hiding this comment.
... I'm fine punting a return to pointers to follow-up work.
Filed as #186.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: crawford, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Returning to the approach we used before 0e65a68 (*: remove etcd nodes, 2018-08-16, openshift#168) to save some copies.
This reverts commit a1b5732. Now that openshift/installer@0e65a68c (*: remove etcd nodes, 2018-08-16, openshift/installer#168) has landed, we can safely remove this.
The OWNERS file is copied from openshift/installer@d22a9870 (OWNERS: Promote abhinavdahiya and wking to approvers, 2018-08-02, openshift/installer#103), which is still current as of openshift/installer@245fc0f6 (Merge pull request openshift/installer#168 from crawford/etcd, 2018-08-27). Copy/pasting into this repo is not very DRY, but this the temporary approach Steve is requesting [1] while we work out something more maintainable. [1]: openshift#1290 (comment)
7 participants
Now that the etcd members have been moved to the master nodes, there is
no need for the etcd nodes.