Managed cluster should ensure control plane operator: Allow hypershift kapi proxy #26579
Conversation
|
/retest-required |
|
/assign bparees |
|
why is this not already a problem? (presumably because the pods fetched by the test in question don't actually include static pods). and if it's a new problem specific to hypershift topologies then i'm not understanding why, when does hypershift create static pods that would fail this test? |
|
@bparees thanks for your feedback. The reason this must be a static pod and can not be a workload that lives in the API is that IBM uses it to loadblance the kubelet -> apiserver connection, so it has to be up before the kubelet contacts the apiserver: openshift/hypershift#685 (review) |
|
i'm still not understanding what is new/novel about the static pods that hypershift is creating, as compared to the static pods that already exist and don't cause this test to fail? |
| @@ -31,7 +31,7 @@ var _ = Describe("[sig-arch] Managed cluster should", func() { | |||
| namespacePrefixes := sets.NewString("kube-", "openshift-") | |||
| excludedNamespaces := sets.NewString("openshift-kube-apiserver", "openshift-kube-controller-manager", "openshift-kube-scheduler", "openshift-etcd", "openshift-openstack-infra", "openshift-ovirt-infra") | |||
| // exclude these pods from checks | |||
| whitelistPods := sets.NewString("network-operator", "dns-operator", "olm-operators", "gcp-routes-controller", "ovnkube-master", "must-gather") | |||
| whitelistPods := sets.NewString("network-operator", "dns-operator", "olm-operators", "gcp-routes-controller", "ovnkube-master", "must-gather", "kube-apiserver-proxy") | |||
There was a problem hiding this comment.
why add the pod name to the whitelist and add logic to generally exclude staticpods?
There was a problem hiding this comment.
That is wrong, I am happy to do either
@bparees nothing, the existing static pods are explicitly excluded instead. I don't really have an opinion on either approach, any favorite? |
i think i prefer the explicit whitelist approach, it avoids the risk that we are ignoring pods we shouldn't be. |
alvaroaleman
force-pushed
the
tolerate
branch
2 times, most recently
from
0f358c2
to
df89212
Compare
alvaroaleman
changed the title
Thanks, updated the change to do that. |
| @@ -32,6 +32,8 @@ var _ = Describe("[sig-arch] Managed cluster should", func() { | |||
| excludedNamespaces := sets.NewString("openshift-kube-apiserver", "openshift-kube-controller-manager", "openshift-kube-scheduler", "openshift-etcd", "openshift-openstack-infra", "openshift-ovirt-infra") | |||
| // exclude these pods from checks | |||
| whitelistPods := sets.NewString("network-operator", "dns-operator", "olm-operators", "gcp-routes-controller", "ovnkube-master", "must-gather") | |||
| // The kube-apiserver proxy exists only in Hypershift | |||
There was a problem hiding this comment.
please give this a full comment on why it is being whitelisted (e.g. the summary from your PR)
…t kapi proxy This pod is used in Hypershift to proxy the connection to the kube apiserver from nodes. As even the kubelet uses it, it has to be a static pod and that results in a keyless noExecute toleration being added by the node. Ref https://issues.redhat.com/browse/HOSTEDCP-257
alvaroaleman
force-pushed
the
tolerate
branch
from
df89212
to
8e8955c
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, bparees 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 |
openshift-ci
bot
added
the
approved
This pod is used in Hypershift to proxy the connection to the kube
apiserver from nodes. As even the kubelet uses it, it has to be a static
pod and that results in a keyless noExecute toleration being added by
the node.
Ref https://issues.redhat.com/browse/HOSTEDCP-257