OCPBUGS-8277: Use a different internal IP for apiserver connectivity by pacevedom · Pull Request #1478 · openshift/microshift

pacevedom · 2023-03-09T14:03:45Z

Which issue(s) this PR addresses:

openshift-ci-robot · 2023-03-09T14:04:20Z

@pacevedom: This pull request references Jira Issue OCPBUGS-8277, which is invalid:

expected the bug to target the "4.14.0" version, but it targets "4.13.0" instead

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

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Which issue(s) this PR addresses:

Closes #

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.

pacevedom · 2023-03-09T14:31:44Z

/jira refresh

pacevedom · 2023-03-09T14:32:27Z

/jira refresh

openshift-ci-robot · 2023-03-09T14:34:03Z

@pacevedom: An error was encountered updating to the POST state for bug OCPBUGS-8277 on the Jira server at https://issues.redhat.com/. No known errors were detected, please see the full error message for details.

Full error message.


Error marking step #27447694 finished: root cause: Tried to update an entity that does not exist.: request failed. Please analyze the request body for more details. Status code: 400:

Please contact an administrator to resolve this issue, then request a bug refresh with /jira refresh.

Details

In response to this:

/jira refresh

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 · 2023-03-09T14:34:07Z

@pacevedom: This pull request references Jira Issue OCPBUGS-8277, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.14.0) matches configured target version for branch (4.14.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jogeo

Details

In response to this:

/jira refresh

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.

dhellmann · 2023-03-09T15:21:09Z

pkg/config/config.go

I'm afraid I don't understand what this section is doing now. Can you give an example, maybe using the default service network settings?

What it does is to get the next immediate subnet from the service CIDR and use that IP.
Given that we need a non-service-CIDR IP to setup for the apiserver endpoint this is the most trivial approach I could think of.
This is also why both parameters (the actual address and the lo interface getting it) are now configurable, since this is an additional IP and there might be cases where we need a different one because of collisions.

Could you expand the comment in the code to explain all of that? The text that is there now might have been clear to someone who understood how we were already choosing an IP, but was not enough for me to understand what was going on.

Ideally, we don't want to assign k8s service IP address to any host interface, because that could cause unexpected problems such as the issue described here: #1478 (comment)

however, it seems that we have to in order to fix the certificate issue: https://issues.redhat.com/browse/OCPBUGS-7442

Given the above, the IP from next service cidr is used in this PR and assigned to lo device.

pacevedom · 2023-03-09T17:57:07Z

/cc @zshi-redhat

dhellmann · 2023-03-09T22:55:01Z

pkg/config/config.go

Why do we want to make the user make this choice? Is this something we can figure out on our own?

Not really, this is a bit complex. You have all these possibilities:

Custom AdvertiseAddress + SkipInterfaceConfiguration=true: This means the api server is either in a different node or is using an already configured interface.

Custom AdvertiseAddress + SkipInterfaceConfiguration=false: This is deliberately ignoring the default next_subnet_after_service_cidr ip, as there might be collisions with that range or simply put it is required to have a different subnet. It does configure the lo interface with the ip.

No AdvertiseAddress + SkipInterfaceConfiguration=true: Same as having a custom advertise address, there might be an interface already configured with the first ip from next_subnet_after_service_cidr.

No AdvertiseAddress + SkipInterfaceConfiguration=false: Default everything, it will configure the first valid ip from next_subnet_after_service_cidr in lo interface. This will be the common case.

Should we make the SkipInterfaceConfiguration an implicit (devel-only) option and assume it is false for single node deployment? because it seems SkipInterfaceConfiguration=true is for multi-node consideration.

Multi node does require this option, however you could also have an IP range you can use in the node and there is no need to configure it for lo interface. I wonder if this is something that could happen.
Also, it defaults to false: https://github.com/openshift/microshift/pull/1478/files#diff-a3d824da3c42420cd5cbb0a4a2c0e7b5bfddd819652788a0596d195dc6e31fa5R251

I'm still unclear on why we need the boolean at all. If the user provides a custom address, we can say that they must pre-configure the interface to use with that address, even if they just use the loopback interface. If they do not provide an address, then we will configure the loopback interface with an address we choose.

So are we ok to say that if not using default then the configuration side of the interface lies on the admin? Having in mind that custom here means "anything different than service-CIDR-next-subnet". If that is ok then I am totally fine to remove the exposed option!

I removed the option as its simpler this way. Thanks!

So are we ok to say that if not using default then the configuration side of the interface lies on the admin? Having in mind that custom here means "anything different than service-CIDR-next-subnet". If that is ok then I am totally fine to remove the exposed option!

Yes, I think that's consistent with how we separate responsibilities for the OS setup in other cases.

Due to the support for IPs in certificates introduced in openshift#1298, the apiserver IP is configured as a secondary address in the lo interface. A VIP is configured by ovnk redirecting 10.43.0.1:443 to 10.43.0.1:6443. 6443 is the port where apiserver listens in the host. 10.43.0.1:443 is used by all pods using client go, as it is computed from the env vars we can find in any pod. If a host network pod or any other tool in the host tries to reach the apiserver by using 10.43.0.1:443 the address is not translated to the endpoint, it tries to contact 10.43.0.1:443 which is not the apiserver but the router. This change computes a new IP endpoint in the next available /32 subnet from the service IP to ensure ovnk does not interfere.

zshi-redhat · 2023-03-10T08:02:47Z

/cc @pliurh PTAL.

Why this matters to ovnk or k8s networking?

10.43.0.1:443 is the k8s apiserver service IP that needs to be accessable by k8s pods (could be hostnetwork pod or pod using CNI overlay network).
when 10.43.0.1 is assigned to lo device, it breaks the traffic flow for hostnetwork pod to apiserver service (e.g. executing curl 10.43.0.1:443 --insecure directly on the host won't reach the apiserver)
this is because the curl cmd finds that 10.43.0.1 is a local host address on lo device so the traffic is sent to lo device directly, instead of being routed to br-ex according to route added by ovnkube-node: 10.43.0.0/16 via 169.254.169.4 dev br-ex mtu 1400.
when traffic sent to br-ex, ovnk DNATs the virtual service IP to the backend IP (<node-ip>:6443, notice the port is 6443) which is the port apiserver is listening to.
when traffic sent to lo device directly with port 443, apiserver won't respond to the request
this broken flow doesn't result in failure in bringing up the cluster , because none of the initial pods started by microshift uses this traffic flow.
but if you have a hostnetwork pod (hostNetwork: true) tries to access virtual service IP 10.43.0.1:443, it fails.

dhellmann · 2023-03-11T18:54:11Z

@pacevedom maybe we could take #1442 before this one? It should be easy to add a method to report when the user has provided a value for the IP and update the interface management logic based on that.

openshift-ci-robot · 2023-03-11T19:09:08Z

@pacevedom: This pull request references Jira Issue OCPBUGS-8277, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.14.0) matches configured target version for branch (4.14.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jogeo

Details

In response to this:

Which issue(s) this PR addresses:

Closes #1460

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.

pliurh · 2023-03-10T09:27:50Z

pkg/config/config.go

This implies that one extra IP address needs to be reserved for microshift. Shall we document it?

Yes, that should be documented.

Added docs commit.

pacevedom · 2023-03-13T08:28:09Z

/retest-required

dhellmann · 2023-03-13T16:11:43Z

/lgtm

I will rebase the config refactoring on top of this.

openshift-ci · 2023-03-13T16:14:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhellmann, pacevedom

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [dhellmann,pacevedom]

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

openshift-ci · 2023-03-13T17:26:17Z

@pacevedom: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

openshift-ci-robot · 2023-03-13T17:29:51Z

@pacevedom: Jira Issue OCPBUGS-8277: All pull requests linked via external trackers have merged:

openshift/microshift#1478

Jira Issue OCPBUGS-8277 has been moved to the MODIFIED state.

Details

In response to this:

Which issue(s) this PR addresses:

Closes #1460

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.

pacevedom · 2023-03-14T07:02:51Z

/cherry-pick release-4.13

openshift-cherrypick-robot · 2023-03-14T07:03:33Z

@pacevedom: new pull request created: #1501

Details

In response to this:

/cherry-pick release-4.13

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.

pacevedom changed the title ~~Ocpbugs 8277~~ OCPBUGS-8277: Use a different internal IP for apiserver connectivity Mar 9, 2023

openshift-ci bot requested review from oglok and sallyom March 9, 2023 14:05

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 9, 2023

pacevedom force-pushed the OCPBUGS-8277 branch from ac530f8 to 9b1da2b Compare March 9, 2023 14:24

openshift-ci bot requested a review from jogeo March 9, 2023 14:36

dhellmann reviewed Mar 9, 2023

View reviewed changes

openshift-ci bot requested a review from zshi-redhat March 9, 2023 17:57

dhellmann mentioned this pull request Mar 9, 2023

USHIFT-936, USHIFT-942: reconcile the internal and external config data structures #1442

Merged

dhellmann reviewed Mar 9, 2023

View reviewed changes

dhellmann mentioned this pull request Mar 9, 2023

[BUG] Cannot validate certificate for 10.43.0.1 because it doesn't contain any IP SANs #1460

Closed

pacevedom force-pushed the OCPBUGS-8277 branch from 9b1da2b to b1964a1 Compare March 9, 2023 22:56

pacevedom force-pushed the OCPBUGS-8277 branch from b1964a1 to cf6aa36 Compare March 10, 2023 15:47

pliurh reviewed Mar 13, 2023

View reviewed changes

OCPBUGS-8277: Update docs

c2129c2

pacevedom mentioned this pull request Mar 13, 2023

MicroShift: update apiserver certificates configuration + networking openshift/enhancements#1366

Merged

openshift-ci bot assigned dhellmann Mar 13, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 13, 2023

openshift-merge-robot merged commit c531355 into openshift:main Mar 13, 2023

openshift-cherrypick-robot mentioned this pull request Mar 14, 2023

[release-4.13] OCPBUGS-9965: Use a different internal IP for apiserver connectivity #1501

Merged

pacevedom deleted the OCPBUGS-8277 branch December 18, 2023 22:28

Conversation

pacevedom commented Mar 9, 2023 • edited by dhellmann Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Mar 9, 2023

Uh oh!

pacevedom commented Mar 9, 2023

Uh oh!

pacevedom commented Mar 9, 2023

Uh oh!

openshift-ci-robot commented Mar 9, 2023

Uh oh!

openshift-ci-robot commented Mar 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pacevedom commented Mar 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pacevedom Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pacevedom Mar 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pacevedom Mar 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zshi-redhat commented Mar 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhellmann commented Mar 11, 2023

Uh oh!

openshift-ci-robot commented Mar 11, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pacevedom commented Mar 13, 2023

Uh oh!

dhellmann commented Mar 13, 2023

Uh oh!

openshift-ci bot commented Mar 13, 2023

Uh oh!

openshift-ci bot commented Mar 13, 2023

Uh oh!

openshift-ci-robot commented Mar 13, 2023

Uh oh!

pacevedom commented Mar 14, 2023

Uh oh!

openshift-cherrypick-robot commented Mar 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

pacevedom commented Mar 9, 2023 •

edited by dhellmann

Loading

pacevedom Mar 9, 2023 •

edited

Loading

pacevedom Mar 10, 2023 •

edited

Loading

pacevedom Mar 10, 2023 •

edited

Loading

zshi-redhat commented Mar 10, 2023 •

edited

Loading