Fix TestFileIntegrityCertRotation e2e test #440
Conversation
openshift-ci
bot
added
the
approved
Vincent056
force-pushed
the
rotate_cert
branch
from
7bea2da
to
eb325c7
Compare
Vincent056
force-pushed
the
rotate_cert
branch
2 times, most recently
from
016cbe5
to
959d9fe
Compare
|
/retest Looks like the e2e setup failed due to |
|
/retest |
|
failed due to ImagePullBackoff |
|
/retest |
Vincent056
force-pushed
the
rotate_cert
branch
2 times, most recently
from
dd16976
to
a9be543
Compare
|
TestFileIntegrityCertRotation is fixed, however there are issue with node taint on bundle e2e test |
Change the how we check if a kubelet cert rotation is being completed, and make sure FIO object pass after the rotation
Vincent056
force-pushed
the
rotate_cert
branch
from
a9be543
to
922ebf5
Compare
|
@Vincent056: all tests passed! Full PR test history. Your PR dashboard. 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. |
| if secret.Annotations == nil { | ||
| secret.Annotations = map[string]string{} | ||
| // wait for cert rotation daemonset to be ready | ||
| err = waitForDaemonSet(daemonSetIsReady(f.KubeClient, "kubelet-bootstrap-cred-manager", "openshift-machine-config-operator")) |
There was a problem hiding this comment.
nit: Potential area for improvement (although I don't expect us to do it in this patch), would be to consider a simplified polling method here, instead of a nested one:
name := "kubelet-bootstrap-cred-manager"
namespace := "openshift-machine-config-operator"
err = assertDaemonSetIsActive(name, namespace)
| t.Errorf("Timed out waiting for DaemonSet kubelet-bootstrap-cred-manager") | ||
| } | ||
|
|
||
| // Delete the secrets csr-signer-signer and csr-signer from the openshift-kube-controller-manager-operator namespace |
There was a problem hiding this comment.
We do this because it's what forces the rotation, right?
| } | ||
| taintedNode.Spec.Taints = append(taintedNode.Spec.Taints, taint) | ||
| t.Logf("Tainting node: %s", taintedNode.Name) | ||
|
|
There was a problem hiding this comment.
Interesting - so what this causing issues in previous CI runs because it wasn't being retried? Or we're working around the CI issue because we're retrying?
| @@ -1755,38 +2030,45 @@ func waitUntilPodsAreGone(t *testing.T, c client.Client, pods *corev1.PodList, i | |||
| } | |||
|
|
|||
| func taintNode(t *testing.T, f *framework.Framework, node *corev1.Node, taint corev1.Taint) error { | |||
There was a problem hiding this comment.
nit: Taints are specific to nodes, we could simplify the name to addTaint.
| taintedNode.Spec.Taints = []corev1.Taint{} | ||
| } | ||
| taintedNode.Spec.Taints = append(taintedNode.Spec.Taints, taint) | ||
| t.Logf("Tainting node: %s", taintedNode.Name) | ||
| return f.Client.Update(goctx.TODO(), taintedNode) | ||
| }, | ||
| ) | ||
| } | ||
|
|
||
| func removeNodeTaint(t *testing.T, f *framework.Framework, nodeName, taintKey string) error { |
There was a problem hiding this comment.
nit: Similar comment as above, we could simplify to removeTaint
| return retryDefault( | ||
| func() error { | ||
| // taint with retry | ||
| // let's fetch the latest node object first | ||
| fetchedNode := &corev1.Node{} |
There was a problem hiding this comment.
Do we need to fetch the node again, even though it's passed in, because we're not sure if it has been updated previously and cause our update to fail because we're attempting to update an older version?
There was a problem hiding this comment.
I only have minor suggestions inline, most of which we can address in subsequent patches if we decide to do so.
Thanks for fixing up the CI, @Vincent056
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhmdnd, Vincent056 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 |
Change the how we check if a kubelet cert rotation is being completed, and make sure FIO object pass after the rotation