Add OpenStack credentials request - #211
Conversation
openshift-ci-robot
added
the
size/S
7c22475 to
ff05d20
Compare
|
Seems reasonable, but do we have any kind of chicken-and-egg problem? |
|
Yeah, we can't use the cloud-credential-operator. We need to work with creds directly (which is not a problem because we have all the privileges). I know for AWS installs, there's a Secret containing the top-level AWS cred, so we can just request that secret. I assume it works the same way for other cloud providers. |
|
(ie, just look at how the cloud-credential-operator gets its creds, and do the same thing here) |
|
@squeed, @danwinship: Oh, you're right here. If you take a look into kuryr_bootstrap.go you can see that we're already getting the secret the way you described and this PR was supposed to get this ready for cluster-credential-operator introduction, but you're right that it won't be able to start without CNO. @dgoodwin, can you advice if it's intended that CNO should not use cloud-credentials-operator? |
|
Can you summarize the issue, we thought the cred operator was up pretty early. |
|
@dgoodwin: Sure thing! cluster-network-operator is responsible for deploying and configuring an SDN - openshift-sdn by default, but it also supports kuryr-kubernetes for OpenStack installations or ovn-kubernetes. The SDN is basically a CNI plugin, that kubelet calls in order to request network wiring for a pod. The way most (all?) CNI plugins are deployed on K8s/OpenShift is as a DaemonSet that will run on all kubelet nodes, with pods that have CNI binaries and configuration directories from the host mounted and are supposed to inject its own binary and configuration there for kubelet to use. Thing is - no pod can start before CNI plugins are configured, because it won't get network. To work this around all the pods related to networking (cluster-network-operator, all openshift-sdn pods, etc.) are run with hostNetworking making sure they bypass that step and can be started even without a CNI plugin. cluster-credentials-operator does not use hostNetworking, meaning that it will need to wait for CNI to be configured in order to start. Until that happens kubelet will simply retry calling CNI in a loop. If cluster-network-operator will rely on cluster-credentials-operator, we're getting a deadlock. |
|
I'm not familiar with the implications, is the problem solved if we use hostNetworking in the cred operator? |
|
@dgoodwin: Yeah, that would definitely solve the problem. The only question is do we want to do it? Is it wrong that cluster-network-operator just reaches the credential secrets directly? I don't really have strong opinions as I wasn't participating in design discussions and I'm probably unaware of all the assumptions. |
|
Sadly, it's not so simple :-). There are basically no other operators that come up before the CNO. So, if you want to come up before the CNO, you can't depend on:
So, it probably doesn't make sense for the CNO to rely on the CCO. If there was a way to bootstrap the cluster without CCO credentials, then to pivot after-the-fact, then that would work. |
|
We might meet those requirements to be honest. Bootstrapping with root creds and pivoting also sounds fairly good. The more we get under the purview of the cred operator the better. There were some use cases we hoped for the future where root creds might never touch the cluster itself. CC @joelddiaz @crawford @abhinavdahiya we should probably talk about this on an arch call maybe this Thu. |
|
What are these creds used to do, and is there a parallel for AWS? Wondering if you use AWS root creds today. |
|
I'll let @dulek chime in on exactly what the credentials are used for in this case. We will need something similar for AWS (and all other platforms) to set up pod networking. The goal is to move that out of the installer. So, think any SGs needed to enable, say, vxlan. Maybe other things as well. |
|
From Group G arch call today, we're going to try to get the credentials operator up and running as a static pod on the bootstrap node, so you will be able to mint creds as early as you need to. Tracking here: https://jira.coreos.com/secure/RapidBoard.jspa?rapidView=107&projectKey=CO&view=planning&selectedIssue=CO-502 |
Neat! Thanks. |
|
@dgoodwin: Any estimate on when is that change planned? I'm just asking to know if we can just fetch root secret for 4.2 and care about it later? Or will it happen in 4.2? |
|
I've got it in this sprint but I'm not sure we'll get to the full implementation in time. I'd assume 2 sprints. Root cred for now is probably the best bet. |
|
Reviving this. @dgoodwin, did you manage to do the static pod trickery? |
dgoodwin is out on pto this week, but he did ask me to look into this while he was out. I've just finished up the last things I had on my plate from last week, so I'm planning to start looking into this today. I'll update here when there's something to share. |
|
We haven't been able to get this implemented in time for 4.2 and looking at it I suspect it's going to require an exception, as it's not really a bug, and it feels like it may not be an easy sell. How critical is this for 4.2 or should we defer to 4.3? Jira with Joel's details here: https://jira.coreos.com/browse/CO-502 |
@dgoodwin From Kuryr side this is not critical for 4.2, as we are already relying on directly retrieving the OpenStack credentials secret. |
Kuryr is a CNI plugin that provides networking to Pods and Services through OpenStack Neutron and OpenStack Octavia. This is desirable when OpenShift is running on OpenStack as avoiding double encapsulation (OpenStack SDN + OpenShift SDN) is a performance improvement. To achieve that Kuryr needs access to APIs of the underlying OpenStack cloud in order to create networks, subnets, ports (ifaces) and loadbalancers for the OpenShift resources. That obviously requires clouds credentials to be part of Kuryr configuration that CNO prepares (CNO itself needs those as well as it creates some OpenStack resources to "bootstrap" Kuryr). Currently we're just reading the root secret, i.e. |
|
hey @dulek can we close this? I think so. |
|
I'm pretty sure this is not required for 4.3 (at least nothing will break without this in 4.3), but I don't know how it'll be going forward. @dgoodwin, I see https://jira.coreos.com/browse/CO-502 is done now, meaning that CCO will run on host networking and before cluster-network-operator is up? |
|
cc @joelddiaz did we get the host network part? It's done on the cred operator, IIRC we're still waiting for a dependent installer PR to merge. |
Sorry for the slow reply. Yes, host networking https://github.com/openshift/cloud-credential-operator/blob/master/cmd/manager/render.go#L51 . And the PR did merge recently openshift/installer#2447 |
Hey, @MaysaMacedo, can you take a look if we can revive this? Running an installation with this will probably be enough to verify it. |
Sure. I will run one installation with it. |
This commit adds openstack credentials request and its support in Kuryr.
ff05d20 to
a63bb89
Compare
|
/test e2e-gcp-upgrade |
|
/lgtm We tested that it works. @squeed, @danwinship, this seems good to go! |
|
Just to clarify, the cred operator changes to run on bootstrap node early enough is working and able to get you the creds you need? |
It does seem so. |
|
Awesome, thanks! Kudos @joelddiaz :) |
|
/test e2e-gcp |
|
Is this going to cause any problems for non-openstack clusters? It seems we're always creating the credentials request. If not, then we're good. If yes, then we need to be a bit cleverer? |
@squeed For non-OpenStack clusters the credentials requests will be ignored. As an example, the image-registry-operator always creates the credentials requests for AWS, Azure, GCS and OpenStack [1], and the credentials operator ignores the requests not supported [2]. [1] https://github.com/openshift/cluster-image-registry-operator/tree/master/manifests |
|
Great. Then, one final question: in the bootstrap code, what happens if we "win the race" and the credentials don't exist yet (but they eventually will). Will it just error and retry? |
@squeed Yes. An error will happen when fetching the secret, and it will be retried. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dulek, MaysaMacedo, squeed 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 |
openshift-ci-robot
added
the
approved
|
/test e2e-gcp-upgrade |
9 participants
This commit adds OpenStack credentials request and its support in Kuryr.