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.

Sign up for GitHub

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

Jump to bottom

Bug 1805254: openshift-apiserver should directly use the host-etcd IPs #312

Merged

openshift-merge-robot merged 1 commit into openshift:master from sanchezl:use-etcd-host-ip-not-service

Feb 20, 2020

Contributor

sanchezl commented Feb 19, 2020

No description provided.

openshift-ci-robot added the size/L label

openshift-ci-robot requested review from deads2k and soltysh

February 19, 2020 17:27

sanchezl force-pushed the use-etcd-host-ip-not-service branch from 3d281f3 to 562bf2c Compare

February 19, 2020 18:19

sanchezl changed the title ~~observe storage urls from host-etcd endpoint address ips~~ Bug 1804700: openshift-apiserver should directly use the host-etcd IPs

openshift-ci-robot added the bugzilla/invalid-bug label

openshift-ci-robot commented Feb 19, 2020

@sanchezl: This pull request references Bugzilla bug 1804700, which is invalid:

expected the bug to target the "4.5.0" release, but it targets "4.4.0" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1804700: openshift-apiserver should directly use the host-etcd IPs

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.

deads2k reviewed

View reviewed changes

pkg/operator/configobservation/etcd/observe_etcd.go Outdated

		@@ -0,0 +1,89 @@
		package etcd

Contributor

deads2k Feb 19, 2020

etcdobserver

Contributor Author

sanchezl Feb 20, 2020

Done

deads2k reviewed

View reviewed changes

pkg/operator/configobservation/etcd/observe_etcd.go Outdated


		"k8s.io/apimachinery/pkg/api/errors"
		"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

Contributor

deads2k Feb 19, 2020

no spaces

Contributor Author

sanchezl Feb 20, 2020

Done

deads2k reviewed

View reviewed changes

pkg/operator/configobservation/etcd/observe_etcd.go Outdated

+              // ObserveStorageURLs observes the storage config URLs. If there is a problem observing the current storage config URLs,
+              // then the previously observed storage config URLs will be re-used.
+              func ObserveStorageURLs(genericListers configobserver.Listers, recorder events.Recorder, currentConfig map[string]interface{}) (observedConfig map[string]interface{}, errs []error) {

Contributor

deads2k Feb 19, 2020

no named return values.

Contributor Author

sanchezl Feb 20, 2020

Done

deads2k reviewed

View reviewed changes

pkg/operator/configobservation/etcd/observe_etcd.go Outdated

+              				errs = append(errs, ipErr)
+              				continue
+              			}
+              			if ip.To4() != nil {

Contributor

deads2k Feb 19, 2020

there is no order. Add a check to make sure these aren't in the 192.whatever.the.playspace.value.is

Contributor Author

sanchezl Feb 19, 2020 •

edited

there is no order

What are you trying to say?

Add a check to make sure these aren't in the 192.whatever.the.playspace.value.is

consider that an error?

Contributor Author

sanchezl Feb 20, 2020

Done

deads2k reviewed

View reviewed changes

pkg/operator/configobservation/etcd/observe_etcd.go Outdated

+              	}
+              	if len(etcdURLs) == 0 {
+              		emptyURLErr := fmt.Errorf("endpoints %s/%s: no etcd endpoint addresses found", etcdEndpointNamespace, etcdEndpointName)

Contributor

deads2k Feb 19, 2020

send event and cleanly set nothing to allow the default service hostname to work.

Contributor

deads2k Feb 19, 2020

send event and cleanly set nothing to allow the default service hostname to work.

this will happen continuously until the etcd operator is updated

Contributor Author

sanchezl Feb 20, 2020

Done

sanchezl force-pushed the use-etcd-host-ip-not-service branch from 562bf2c to 6c5f12f Compare

February 19, 2020 21:32

sttts reviewed

View reviewed changes

pkg/operator/configobservation/etcdobserver/observe_etcd.go Outdated

+              	etcdEndpoints, err := listers.EndpointsLister.Endpoints(etcdEndpointNamespace).Get(etcdEndpointName)
+              	if errors.IsNotFound(err) {
+              		recorder.Warningf("ObserveStorageFailed", "Required %s/%s endpoint not found", etcdEndpointNamespace, etcdEndpointName)
+              		errs = append(errs, fmt.Errorf("endpoints/host-etcd.kube-system: not found"))

Contributor

sttts Feb 20, 2020

use constants in this error?

Contributor Author

sanchezl Feb 20, 2020

Done
Cleaned up all messages (originally copied from kao).

sttts reviewed

View reviewed changes

pkg/operator/configobservation/etcdobserver/observe_etcd.go

+              				errs = append(errs, ipErr)
+              				continue
+              			}
+              			if strings.HasPrefix(ip.String(), "192.0.2.") || strings.HasPrefix(ip.String(), "2001:db8:") {

Contributor

sttts Feb 20, 2020

what are these? Needs comment.

Contributor

deads2k Feb 20, 2020

standard documentation cidrs

Contributor Author

sanchezl Feb 20, 2020

Done
Comment added.

sttts reviewed

View reviewed changes

pkg/operator/configobservation/etcdobserver/observe_etcd_test.go Outdated

+              	}
+              }
+              func getWantObserverConfig(etcdURLs []string) (map[string]interface{}, error) {

Contributor

sttts Feb 20, 2020

getWant?

Contributor Author

sanchezl Feb 20, 2020

Done.
Deleted. Dead code from initial copy from kao.


          observe storage urls from host-etcd endpoint address ips

ab08c50

sanchezl force-pushed the use-etcd-host-ip-not-service branch from 6c5f12f to ab08c50 Compare

February 20, 2020 15:04

sanchezl changed the title ~~Bug 1804700: openshift-apiserver should directly use the host-etcd IPs~~ Bug 1805254: openshift-apiserver should directly use the host-etcd IPs

openshift-ci-robot added bugzilla/valid-bug and removed bugzilla/invalid-bug labels

openshift-ci-robot commented Feb 20, 2020

@sanchezl: This pull request references Bugzilla bug 1805254, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Bug 1805254: openshift-apiserver should directly use the host-etcd IPs

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.

Contributor

deads2k commented Feb 20, 2020

/lgtm

/cherrypick release-4.4

openshift-cherrypick-robot commented Feb 20, 2020

@deads2k: once the present PR merges, I will cherry-pick it on top of release-4.4 in a new PR and assign it to you.

In response to this:

/lgtm

/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.

openshift-ci-robot assigned deads2k

openshift-ci-robot added the lgtm label

openshift-ci-robot commented Feb 20, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, sanchezl

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:

~~OWNERS~~ [deads2k]

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 added the approved label

Contributor

openshift-bot commented Feb 20, 2020

/retest

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

deads2k mentioned this pull request

etcdservicecontroller: observe etcd endpoints #300

Closed

Contributor

openshift-bot commented Feb 20, 2020

/retest

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

1 similar comment

Contributor

openshift-bot commented Feb 20, 2020

/retest

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

openshift-merge-robot merged commit 00fc5d5 into openshift:master

openshift-cherrypick-robot mentioned this pull request

[release-4.4] Bug 1805534: openshift-apiserver should directly use the host-etcd IPs #315

Merged

openshift-cherrypick-robot commented Feb 20, 2020

@deads2k: new pull request created: #315

In response to this:

/lgtm

/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.

openshift-ci-robot commented Feb 20, 2020

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

In response to this:

Bug 1805254: openshift-apiserver should directly use the host-etcd IPs

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment