[occm] consider private network names when node contains IPs from other private networks #1041

kayrus · 2020-04-20T12:39:42Z

What this PR does / why we need it:

If there is a private network name specified for nodes and the IP list contains IPs from a different private network, they have to be ignored by OCCM.

Which issue this PR fixes(if applicable):
fixes #1039

Special notes for reviewers:
Cherry-pick candidate for branch release-1.18

Release note:

None

@gdetal try this PR in your environment and let me know if it works fine for your case.

k8s-ci-robot · 2020-04-20T12:39:50Z

Hi @kayrus. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

kayrus · 2020-04-20T12:43:18Z

/assign @lingxiankong

theopenlab-ci · 2020-04-20T12:43:51Z

Build succeeded.

cloud-provider-openstack-format : SUCCESS in 3m 26s

theopenlab-ci · 2020-04-20T12:44:31Z

Build failed.

cloud-provider-openstack-unittest : FAILURE in 4m 00s

theopenlab-ci · 2020-04-20T12:53:16Z

Build succeeded.

cloud-provider-openstack-format : SUCCESS in 3m 15s

theopenlab-ci · 2020-04-20T12:53:33Z

Build failed.

cloud-provider-openstack-unittest : FAILURE in 4m 05s

theopenlab-ci · 2020-04-20T13:03:10Z

Build succeeded.

cloud-provider-openstack-acceptance-test-lb-octavia : SUCCESS in 13m 20s

theopenlab-ci · 2020-04-20T13:06:06Z

Build failed.

cloud-provider-openstack-acceptance-test-csi-cinder : FAILURE in 16m 00s

gdetal · 2020-04-20T13:20:13Z

@kayrus I can confirm that this fixes #1039

I0420 13:17:47.881870       1 openstack.go:754] Node 'openstack-md-0-h6z75' address '10.8.0.235' ignored due to 'internal-network-name' option
I0420 13:17:47.881886       1 openstack.go:754] Node 'openstack-md-0-h6z75' address '10.7.0.74' ignored due to 'internal-network-name' option
I0420 13:17:47.881890       1 openstack.go:754] Node 'openstack-md-0-h6z75' address '10.9.0.94' ignored due to 'internal-network-name' option
I0420 13:17:47.881896       1 openstack_instances.go:90] NodeAddresses(openstack-md-0-h6z75) => [{InternalIP 10.6.0.199}]

Thanks !

theopenlab-ci · 2020-04-20T13:31:36Z

Build succeeded.

cloud-provider-openstack-acceptance-test-csi-manila : SUCCESS in 41m 13s

theopenlab-ci · 2020-04-20T14:40:51Z

Build succeeded.

cloud-provider-openstack-acceptance-test-e2e-conformance : SUCCESS in 1h 50m 06s

ramineni · 2020-04-21T05:05:26Z

/ok-to-test

k8s-ci-robot · 2020-04-21T12:44:30Z

@kayrus: The /retest command does not accept any targets.
The following commands are available to trigger jobs:

/test pull-cloud-provider-openstack-check
/test pull-cloud-provider-openstack-test

Use /test all to run all jobs.

In response to this:

/retest cloud-provider-openstack-acceptance-test-csi-cinder

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.

k8s-ci-robot · 2020-04-21T12:44:41Z

@kayrus: The /retest command does not accept any targets.
The following commands are available to trigger jobs:

/test pull-cloud-provider-openstack-check
/test pull-cloud-provider-openstack-test

Use /test all to run all jobs.

In response to this:

/retest cloud-provider-openstack-unittest

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.

kayrus · 2020-04-21T12:45:30Z

test cloud-provider-openstack-acceptance-test-csi-cinder

kayrus · 2020-04-21T12:45:42Z

test cloud-provider-openstack-unittest

kayrus · 2020-04-21T12:46:10Z

retest cloud-provider-openstack-acceptance-test-csi-cinder

kayrus · 2020-04-21T12:46:34Z

/retest cloud-provider-openstack-unittest

k8s-ci-robot · 2020-04-21T12:46:47Z

@kayrus: The /retest command does not accept any targets.
The following commands are available to trigger jobs:

/test pull-cloud-provider-openstack-check
/test pull-cloud-provider-openstack-test

Use /test all to run all jobs.

In response to this:

/retest cloud-provider-openstack-unittest

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.

theopenlab-ci · 2020-04-25T06:39:10Z

Build succeeded.

cloud-provider-openstack-unittest : SUCCESS in 3m 48s

theopenlab-ci · 2020-04-25T06:52:35Z

Build succeeded.

cloud-provider-openstack-acceptance-test-lb-octavia : SUCCESS in 17m 05s

theopenlab-ci · 2020-04-25T07:05:03Z

Build succeeded.

cloud-provider-openstack-acceptance-test-csi-cinder : SUCCESS in 28m 49s

theopenlab-ci · 2020-04-25T07:24:37Z

Build succeeded.

cloud-provider-openstack-acceptance-test-csi-manila : SUCCESS in 47m 22s

theopenlab-ci · 2020-04-25T08:40:59Z

Build failed.

cloud-provider-openstack-acceptance-test-e2e-conformance : FAILURE in 2h 04m 34s

kayrus · 2020-04-25T16:37:39Z

/test cloud-provider-openstack-acceptance-test-e2e-conformance

k8s-ci-robot · 2020-04-25T16:37:53Z

@kayrus: The specified target(s) for /test were not found.
The following commands are available to trigger jobs:

/test pull-cloud-provider-openstack-check
/test pull-cloud-provider-openstack-test

Use /test all to run all jobs.

In response to this:

/test cloud-provider-openstack-acceptance-test-e2e-conformance

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.

theopenlab-ci · 2020-04-25T18:30:31Z

Build succeeded.

cloud-provider-openstack-acceptance-test-e2e-conformance : SUCCESS in 1h 52m 15s

lingxiankong · 2020-04-25T22:13:38Z

pkg/cloudprovider/providers/openstack/openstack.go

 				addressType = v1.NodeExternalIP
+			} else if util.Contains(networkingOpts.PublicNetworkName, network) {
+				addressType = v1.NodeExternalIP
+				RemoveFromNodeAddresses(&addrs,


Why the address is removed here if the network name is included in PublicNetworkName?

@zuzzas could you please explain your test case?

It may be present in the fixed IPs and erroneously counted as an InternalIP. We remove it and add again as a proper ExternalIP address.

@zuzzas I'm still confused, in which case It may be present in the fixed IPs and erroneously counted as an InternalIP?

Without the proposed patch, in the following situation, 10.220.213.38 will be counted both as InternalIP and ExternalIP. It does not look right, since we've explicitly designated in the CCM's configuration the network BGP-VLAN-3894 with this IP as public.

$ openstack --debug server show f4a319e6-95fb-445a-ac2d-a6374264d6c1

{ "server": { "addresses": { "BGP-VLAN-3894": [ { "OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:42:95:9f", "version": 4, "addr": "10.220.213.157", "OS-EXT-IPS:type": "fixed" } ], "e2e": [ { "OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:6c:f6:bb", "version": 4, "addr": "192.168.199.52", "OS-EXT-IPS:type": "fixed" } ] } } }

$ nova --debug interface-list f4a319e6-95fb-445a-ac2d-a6374264d6c1

{ "interfaceAttachments": [ { "port_id": "08368bb9-1021-4f08-a318-44bade127942", "port_state": "ACTIVE", "tag": null, "mac_addr": "fa:16:3e:42:95:9f", "fixed_ips": [ { "subnet_id": "728ca9da-da11-4076-ac32-c47a18693c5c", "ip_address": "10.220.213.157" } ], "net_id": "a30dbaf7-dd86-4c67-aaa5-42c4fdf33e2a" }, { "port_id": "8cc3d233-89ae-4e06-aed8-5b92cea3b5a4", "port_state": "ACTIVE", "tag": null, "mac_addr": "fa:16:3e:6c:f6:bb", "fixed_ips": [ { "subnet_id": "b33a5f97-3924-4f8d-ad8c-10d5b08f81aa", "ip_address": "192.168.199.52" } ], "net_id": "847533ef-13bd-4655-a9b3-812b5ba1e3e0" } ] }

Does BGP-VLAN-3894 exist in the output of openstack network list --external command?

aks@aks-ThinkPad-X230:~$ openstack network list --external +--------------------------------------+---------------+--------------------------------------+ | ID | Name | Subnets | +--------------------------------------+---------------+--------------------------------------+ | a30dbaf7-dd86-4c67-aaa5-42c4fdf33e2a | BGP-VLAN-3894 | 728ca9da-da11-4076-ac32-c47a18693c5c | | c153f6b2-c186-4828-b98c-9cbd79f13621 | VLAN-3502 | 6dc455a0-f44c-4d9a-9d10-8557c4d38f53 | +--------------------------------------+---------------+--------------------------------------+```

@zuzzas that's pretty weird, according to the console output you provided above,

"BGP-VLAN-3894": [ { "OS-EXT-IPS-MAC:mac_addr": "fa:16:3e:42:95:9f", "version": 4, "addr": "10.220.213.157", "OS-EXT-IPS:type": "fixed" } ],

BGP-VLAN-3894 should be an internal Neutron network.

Nevermind, it's not harmful to add that logic, do you mind adding some comments for Line 751 before we merge this PR?

I'd launch a full-blown investigation into this peculiarity. Alas, it is a public OpenStack from Mail.ru Cloud Solutions.

Sure.

It is done.

Added comment

theopenlab-ci · 2020-04-30T12:36:07Z

Build succeeded.

cloud-provider-openstack-format : SUCCESS in 3m 14s

theopenlab-ci · 2020-04-30T12:36:20Z

Build succeeded.

cloud-provider-openstack-unittest : SUCCESS in 3m 30s

theopenlab-ci · 2020-04-30T12:44:31Z

Build succeeded.

cloud-provider-openstack-acceptance-test-lb-octavia : SUCCESS in 11m 30s

theopenlab-ci · 2020-04-30T13:01:08Z

Build succeeded.

cloud-provider-openstack-acceptance-test-csi-cinder : SUCCESS in 27m 13s

theopenlab-ci · 2020-04-30T13:08:09Z

Build succeeded.

cloud-provider-openstack-acceptance-test-csi-manila : SUCCESS in 34m 11s

lingxiankong · 2020-04-30T21:47:01Z

/lgtm

Thanks!

ramineni · 2020-05-01T06:11:43Z

/approve

k8s-ci-robot · 2020-05-01T06:11:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ramineni

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~~ [ramineni]

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

kayrus · 2020-05-01T09:00:45Z

@zuzzas, thanks for this PR

…er private networks (kubernetes#1041) * OCCM: consider network names, when specified * Move v1 helper functions into openstack_instances.go * [occm] Address detection logic fixes Signed-off-by: Andrey Klimentyev <andrey.klimentyev@flant.com> * Typo fix * Move LB related helpers into occm package * Added comment Co-authored-by: Andrey Klimentyev <andrey.klimentyev@flant.com>

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 20, 2020

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 20, 2020

k8s-ci-robot requested review from FengyunPan2 and mrhillsman April 20, 2020 12:40

k8s-ci-robot assigned lingxiankong Apr 20, 2020

OCCM: consider network names, when specified

7aa1b83

kayrus force-pushed the handle-net-names branch from 6fec385 to 7aa1b83 Compare April 20, 2020 12:47

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 21, 2020

lingxiankong reviewed Apr 25, 2020

View reviewed changes

zuzzas and others added 2 commits April 30, 2020 15:01

Added comment

8377cb2

Merge pull request #2 from flant/kayrus-pr

9d924d3

Added comment

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 30, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 1, 2020

k8s-ci-robot merged commit c544421 into kubernetes:master May 1, 2020

kayrus deleted the handle-net-names branch May 1, 2020 08:59

lingxiankong mentioned this pull request May 3, 2020

[occm] consider private network name when node contains IPs from other private networks #1053

Merged

deshui123 mentioned this pull request Aug 4, 2020

cloud controller is unable to patch node with IP addresses from cloud provider kubernetes/cloud-provider-vsphere#368

Closed

kayrus mentioned this pull request Oct 7, 2020

Multi-nic node ip must correspond to kubelet api listening address #407

Closed

kayrus mentioned this pull request Jan 15, 2021

compute_instance_v2: handle special case of 2 default networks terraform-provider-openstack/terraform-provider-openstack#1073

Merged

kayrus mentioned this pull request Mar 1, 2023

REQUEST: New membership for kayrus kubernetes/org#4056

Closed

9 tasks

[occm] consider private network names when node contains IPs from other private networks #1041

[occm] consider private network names when node contains IPs from other private networks #1041

Conversation

kayrus commented Apr 20, 2020 • edited by lingxiankong Loading

k8s-ci-robot commented Apr 20, 2020

kayrus commented Apr 20, 2020

theopenlab-ci bot commented Apr 20, 2020

theopenlab-ci bot commented Apr 20, 2020

theopenlab-ci bot commented Apr 20, 2020

theopenlab-ci bot commented Apr 20, 2020

theopenlab-ci bot commented Apr 20, 2020

theopenlab-ci bot commented Apr 20, 2020

gdetal commented Apr 20, 2020 • edited Loading

theopenlab-ci bot commented Apr 20, 2020

theopenlab-ci bot commented Apr 20, 2020

ramineni commented Apr 21, 2020

k8s-ci-robot commented Apr 21, 2020

k8s-ci-robot commented Apr 21, 2020

kayrus commented Apr 21, 2020

kayrus commented Apr 21, 2020

kayrus commented Apr 21, 2020

kayrus commented Apr 21, 2020

k8s-ci-robot commented Apr 21, 2020

theopenlab-ci bot commented Apr 25, 2020

theopenlab-ci bot commented Apr 25, 2020

theopenlab-ci bot commented Apr 25, 2020

theopenlab-ci bot commented Apr 25, 2020

theopenlab-ci bot commented Apr 25, 2020

kayrus commented Apr 25, 2020

k8s-ci-robot commented Apr 25, 2020

theopenlab-ci bot commented Apr 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zuzzas Apr 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theopenlab-ci bot commented Apr 30, 2020

theopenlab-ci bot commented Apr 30, 2020

theopenlab-ci bot commented Apr 30, 2020

theopenlab-ci bot commented Apr 30, 2020

theopenlab-ci bot commented Apr 30, 2020

lingxiankong commented Apr 30, 2020

ramineni commented May 1, 2020

k8s-ci-robot commented May 1, 2020

kayrus commented May 1, 2020

kayrus commented Apr 20, 2020 •

edited by lingxiankong

Loading

gdetal commented Apr 20, 2020 •

edited

Loading

zuzzas Apr 26, 2020 •

edited

Loading