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

Bug 1813003: etcd: remove etcd DNS entries because etcd no longer uses DNS #3265

Merged
merged 1 commit into from Mar 12, 2020

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Mar 10, 2020

I'm not completely sure where all the DNS bits are, but this is my best starting guess.

Starting in 4.4, etcd does not rely on DNS entries.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 10, 2020
@abhinavdahiya
Copy link
Contributor

Thanks a lot for this @deads2k

/test e2e-azure
/test e2e-gcp

@abhinavdahiya
Copy link
Contributor

I also see https://github.com/deads2k/installer/blob/bdcf138e368e5323eff2eb78f3f9a7d8162123db/pkg/asset/manifests/infrastructure.go#L77 which could be dropped.

Also what about the etcd host endpoint object ? Can that be dropped too?

@iamemilio
Copy link

OpenStack, Baremetal, OVIRT, and Vsphere use the MCO to configure haproxy for dns. We use the same tooling and format, so you will just have to repeat these steps for all the platforms under each of their directory structures. I will use OpenStack as an example:

The main haproxy entry on masters is here: https://github.com/openshift/machine-config-operator/blob/7f52bd8d004aec98768c0b96189f6171b51234a1/templates/master/00-master/openstack/files/openstack-mdns-config.yaml#L8

you will also need to remove this vrrp check from keepalived: https://github.com/openshift/machine-config-operator/blob/ae8e1f47767f4f1302993c2b3b795ac2b9eb4a58/templates/master/00-master/openstack/files/openstack-keepalived-keepalived.yaml#L11

@cybertron did I miss anything?

@abhinavdahiya
Copy link
Contributor

/test e2e-azure

@mandre
Copy link
Member

mandre commented Mar 11, 2020

This should do it for the rest of the platforms (tested successfully with OpenStack): openshift/machine-config-operator#1556

@deads2k
Copy link
Contributor Author

deads2k commented Mar 11, 2020

Also what about the etcd host endpoint object ? Can that be dropped too?

in 4.5, yes. However, I would like to cherrypick this into 4.4, so I'd like to do the 4.5-only work in a different PR.

@deads2k
Copy link
Contributor Author

deads2k commented Mar 11, 2020

I also see https://github.com/deads2k/installer/blob/bdcf138e368e5323eff2eb78f3f9a7d8162123db/pkg/asset/manifests/infrastructure.go#L77 which could be dropped.

Because we support a migration path from 4.3 to 4.4 and we need the operator to be present and running before we can transition to IPs only, we have to maintain the EtcdDiscoveryDomain in 4.4 so that the etcd signing code can remain the same for the migrated and non-migrated cases. In 4.5, we can remove this value and handle the final migration.

Since I'd like to pick this PR into 4.4, I want to do the etcdDiscoveryDomain as a followup.

@abhinavdahiya abhinavdahiya added the platform/baremetal IPI bare metal hosts platform label Mar 11, 2020
@metal3ci
Copy link

Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1610/

@abhinavdahiya
Copy link
Contributor

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 11, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 11, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

4 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 0d36048 into openshift:master Mar 12, 2020
@openshift-ci-robot
Copy link
Contributor

@deads2k: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-azure ce64435 link /test e2e-azure
ci/prow/e2e-libvirt ce64435 link /test e2e-libvirt

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@deads2k
Copy link
Contributor Author

deads2k commented Mar 12, 2020

/cherrypick release-4.4

@openshift-cherrypick-robot

@deads2k: #3265 failed to apply on top of branch "release-4.4":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	data/data/azure/main.tf
M	data/data/bootstrap/files/usr/local/bin/installer-gather.sh
Falling back to patching base and 3-way merge...
Auto-merging data/data/bootstrap/files/usr/local/bin/installer-gather.sh
CONFLICT (content): Merge conflict in data/data/bootstrap/files/usr/local/bin/installer-gather.sh
Auto-merging data/data/azure/main.tf
Patch failed at 0001 etcd: remove etcd DNS entries because etcd no longer uses DNS

In response to this:

/cherrypick release-4.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@retroflexer
Copy link
Contributor

/retitle Bug 1813004: etcd: remove etcd DNS entries because etcd no longer uses DNS

@openshift-ci-robot openshift-ci-robot changed the title etcd: remove etcd DNS entries because etcd no longer uses DNS Bug 1813004: etcd: remove etcd DNS entries because etcd no longer uses DNS Mar 12, 2020
@openshift-ci-robot
Copy link
Contributor

@deads2k: All pull requests linked via external trackers have merged. Bugzilla bug 1813004 has been moved to the MODIFIED state.

In response to this:

Bug 1813004: etcd: remove etcd DNS entries because etcd no longer uses DNS

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@retroflexer
Copy link
Contributor

/retitle Bug 1813003: etcd: remove etcd DNS entries because etcd no longer uses DNS

@openshift-ci-robot openshift-ci-robot changed the title Bug 1813004: etcd: remove etcd DNS entries because etcd no longer uses DNS Bug 1813003: etcd: remove etcd DNS entries because etcd no longer uses DNS Mar 12, 2020
@openshift-ci-robot
Copy link
Contributor

@deads2k: All pull requests linked via external trackers have merged. Bugzilla bug 1813003 has been moved to the MODIFIED state.

In response to this:

Bug 1813003: etcd: remove etcd DNS entries because etcd no longer uses DNS

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mandre added a commit to mandre/machine-config-operator that referenced this pull request Jun 3, 2020
Similar to openshift/installer#3265, this
removes the etcd entries from mdns config because etcd no longer uses
DNS.
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/machine-config-operator that referenced this pull request Jul 9, 2020
Similar to openshift/installer#3265, this
removes the etcd entries from mdns config because etcd no longer uses
DNS.
vrutkovs pushed a commit to vrutkovs/machine-config-operator that referenced this pull request Aug 29, 2020
Similar to openshift/installer#3265, this
removes the etcd entries from mdns config because etcd no longer uses
DNS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. platform/baremetal IPI bare metal hosts platform size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants