-
Notifications
You must be signed in to change notification settings - Fork 200
OCPBUGS-69758:CNTRLPLANE-2222:Migrate go test cert-rotation-tests.go to OTE #1983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| // This file imports test packages to ensure they are included in the build. | ||
| // These imports are necessary to register Ginkgo tests with the OpenShift Tests Extension framework. | ||
| package main | ||
|
|
||
| import ( | ||
| // Import test packages to register Ginkgo tests | ||
| _ "github.com/openshift/cluster-kube-apiserver-operator/test/e2e" | ||
| ) |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,157 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package e2e | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "strings" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "testing" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/stretchr/testify/require" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| corev1 "k8s.io/api/core/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/apimachinery/pkg/api/errors" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/apimachinery/pkg/util/wait" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/client-go/kubernetes" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "k8s.io/utils/clock" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configv1 "github.com/openshift/api/config/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| operatorv1 "github.com/openshift/api/operator/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configclient "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/openshift/cluster-kube-apiserver-operator/pkg/operator" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/operatorclient" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| testlibrary "github.com/openshift/cluster-kube-apiserver-operator/test/library" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configv1helpers "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/openshift/library-go/pkg/operator/genericoperatorclient" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/openshift/library-go/pkg/operator/v1helpers" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| g "github.com/onsi/ginkgo/v2" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var _ = g.Describe("[sig-api-machinery] kube-apiserver operator", func() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| g.It("[Operator][Serial] TestCertRotationTimeUpgradeable", func() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| testCertRotationTimeUpgradeable(g.GinkgoTB()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| g.It("[Operator][Serial] TestCertRotationStompOnBadType", func() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| testCertRotationStompOnBadType(g.GinkgoTB()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func testCertRotationTimeUpgradeable(t testing.TB) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| kubeConfig, err := testlibrary.NewClientConfigForTest() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| operatorClient, _, err := genericoperatorclient.NewStaticPodOperatorClient( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clock.RealClock{}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| kubeConfig, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| operatorv1.GroupVersion.WithResource("kubeapiservers"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| operatorv1.GroupVersion.WithKind("KubeAPIServer"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| operator.ExtractStaticPodOperatorSpec, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| operator.ExtractStaticPodOperatorStatus) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configClient, err := configclient.NewForConfig(kubeConfig) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx := context.Background() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _, operatorStatus, _, err := operatorClient.GetStaticPodOperatorStateWithQuorum(ctx) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.True(t, v1helpers.IsOperatorConditionTrue(operatorStatus.Conditions, "CertRotationTimeUpgradeable")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| kubeClient := kubernetes.NewForConfigOrDie(kubeConfig) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.Logf("Creating unsupported-cert-rotation-config...") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _, err = kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).Create(context.TODO(), &corev1.ConfigMap{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code creates a ctx variable but doesn't use it consistently. But operation uses context.TODO() while others could use the existing ctx. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ObjectMeta: metav1.ObjectMeta{Namespace: operatorclient.GlobalUserSpecifiedConfigNamespace, Name: "unsupported-cert-rotation-config"}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Data: map[string]string{"base": "2y"}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, metav1.CreateOptions{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| defer func() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resource Cleanup: Missing Error Handling |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).Delete(context.TODO(), "unsupported-cert-rotation-config", metav1.DeleteOptions{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code creates a ctx variable but doesn't use it consistently. But operation uses context.TODO() while others could use the existing ctx. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+55
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make ConfigMap create/delete idempotent to reduce flakes @@
t.Logf("Creating unsupported-cert-rotation-config...")
- _, err = kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).Create(context.TODO(), &corev1.ConfigMap{
+ _ = kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).
+ Delete(ctx, "unsupported-cert-rotation-config", metav1.DeleteOptions{})
+ _, err = kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).Create(ctx, &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Namespace: operatorclient.GlobalUserSpecifiedConfigNamespace, Name: "unsupported-cert-rotation-config"},
Data: map[string]string{"base": "2y"},
}, metav1.CreateOptions{})
require.NoError(t, err)
defer func() {
- kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).Delete(context.TODO(), "unsupported-cert-rotation-config", metav1.DeleteOptions{})
+ if err := kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).
+ Delete(ctx, "unsupported-cert-rotation-config", metav1.DeleteOptions{}); err != nil && !errors.IsNotFound(err) {
+ t.Logf("cleanup: failed to delete configmap unsupported-cert-rotation-config: %v", err)
+ }
}()📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err = wait.PollImmediate(1*time.Second, 5*time.Second, func() (bool, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _, operatorStatus, _, err := operatorClient.GetStaticPodOperatorStateWithQuorum(ctx) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clusteroperator, err := configClient.ClusterOperators().Get(context.TODO(), "kube-apiserver", metav1.GetOptions{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code creates a ctx variable but doesn't use it consistently. But operation uses context.TODO() while others could use the existing ctx. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| certRotationCondition := v1helpers.FindOperatorCondition(operatorStatus.Conditions, "CertRotationTimeUpgradeable") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upgradeableCondition := configv1helpers.FindStatusCondition(clusteroperator.Status.Conditions, "Upgradeable") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if certRotationCondition == nil || upgradeableCondition == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, fmt.Errorf("Couldn't find CertRotationTimeUpgradeable or Upgradeable condition") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error Message Capitalization
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if certRotationCondition.Status == operatorv1.ConditionFalse && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upgradeableCondition.Status == configv1.ConditionFalse && strings.Contains(upgradeableCondition.Reason, "CertRotationTime") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.Logf("\nCertRotationTimeUpgradeable: %#v\nUpgradeable: %#v", certRotationCondition, upgradeableCondition) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+71
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Short timeout may cause test flakes in slower environments. The 5-second timeout in both polling loops may be insufficient for end-to-end tests, especially in resource-constrained CI/CD environments where operator reconciliation can be slower. Consider increasing to at least 30-60 seconds to reduce flakiness. Apply this diff to increase the timeouts: - err = wait.PollImmediate(1*time.Second, 5*time.Second, func() (bool, error) {
+ err = wait.PollImmediate(1*time.Second, 30*time.Second, func() (bool, error) {And similarly for the second polling loop: - err = wait.PollImmediate(1*time.Second, 5*time.Second, func() (bool, error) {
+ err = wait.PollImmediate(1*time.Second, 30*time.Second, func() (bool, error) {Additionally, the polling logic is duplicated between lines 71-92 and 99-119 with only minor differences in the conditions checked. Consider extracting a helper function to reduce duplication and improve maintainability. Also applies to: 99-120 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.Logf("Removing unsupported-cert-rotation-config...") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err = kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).Delete(context.TODO(), "unsupported-cert-rotation-config", metav1.DeleteOptions{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err = wait.PollImmediate(1*time.Second, 5*time.Second, func() (bool, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _, operatorStatus, _, err := operatorClient.GetStaticPodOperatorStateWithQuorum(ctx) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clusteroperator, err := configClient.ClusterOperators().Get(context.TODO(), "kube-apiserver", metav1.GetOptions{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| certRotationCondition := v1helpers.FindOperatorCondition(operatorStatus.Conditions, "CertRotationTimeUpgradeable") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upgradeableCondition := configv1helpers.FindStatusCondition(clusteroperator.Status.Conditions, "Upgradeable") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if certRotationCondition == nil || upgradeableCondition == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, fmt.Errorf("Couldn't find CertRotationTimeUpgradeable or Upgradeable condition") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if certRotationCondition.Status == operatorv1.ConditionTrue && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (upgradeableCondition.Status == configv1.ConditionTrue || !strings.Contains(upgradeableCondition.Reason, "CertRotationTime")) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.Logf("\nCertRotationTimeUpgradeable: %#v\nUpgradeable: %#v", certRotationCondition, upgradeableCondition) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func testCertRotationStompOnBadType(t testing.TB) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| kubeConfig, err := testlibrary.NewClientConfigForTest() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| kubeClient := kubernetes.NewForConfigOrDie(kubeConfig) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // this is inherently racy against a controller | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err = wait.PollImmediate(10*time.Millisecond, 5*time.Second, func() (done bool, err error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err := kubeClient.CoreV1().Secrets(operatorclient.OperatorNamespace).Delete(context.TODO(), "aggregator-client-signer", metav1.DeleteOptions{}); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error swallowed
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if _, err := kubeClient.CoreV1().Secrets(operatorclient.OperatorNamespace).Create(context.TODO(), &corev1.Secret{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ObjectMeta: metav1.ObjectMeta{Namespace: operatorclient.OperatorNamespace, Name: "aggregator-client-signer"}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Type: "SecretTypeTLS", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the proper Secret type constant. The string literal Apply this diff: - Type: "SecretTypeTLS",
+ Type: corev1.SecretTypeTLS,📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's should be |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, metav1.CreateOptions{}); err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error swallowed |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| err = wait.PollImmediate(100*time.Millisecond, 30*time.Second, func() (done bool, err error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| curr, err := kubeClient.CoreV1().Secrets(operatorclient.OperatorNamespace).Get(context.TODO(), "aggregator-client-signer", metav1.GetOptions{}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if errors.IsNotFound(err) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, err | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if curr.Type == corev1.SecretTypeTLS { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,146 +1,21 @@ | ||
| package e2e | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "strings" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/api/errors" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/util/wait" | ||
| "k8s.io/client-go/kubernetes" | ||
| "k8s.io/utils/clock" | ||
|
|
||
| configv1 "github.com/openshift/api/config/v1" | ||
| operatorv1 "github.com/openshift/api/operator/v1" | ||
| configclient "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" | ||
| "github.com/openshift/cluster-kube-apiserver-operator/pkg/operator" | ||
| "github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/operatorclient" | ||
| test "github.com/openshift/cluster-kube-apiserver-operator/test/library" | ||
| configv1helpers "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers" | ||
| "github.com/openshift/library-go/pkg/operator/genericoperatorclient" | ||
| "github.com/openshift/library-go/pkg/operator/v1helpers" | ||
| ) | ||
| import "testing" | ||
|
|
||
| // This test calls the shared function which | ||
| // can be called from both standard Go tests and Ginkgo | ||
| // | ||
| // This situation is temporary until we test the new e2e-gcp-operator-serial-ote job. | ||
| // Eventually all tests will be run only as part of the OTE framework. | ||
| func TestCertRotationTimeUpgradeable(t *testing.T) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's add a comment:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
| kubeConfig, err := test.NewClientConfigForTest() | ||
| require.NoError(t, err) | ||
| operatorClient, _, err := genericoperatorclient.NewStaticPodOperatorClient( | ||
| clock.RealClock{}, | ||
| kubeConfig, | ||
| operatorv1.GroupVersion.WithResource("kubeapiservers"), | ||
| operatorv1.GroupVersion.WithKind("KubeAPIServer"), | ||
| operator.ExtractStaticPodOperatorSpec, | ||
| operator.ExtractStaticPodOperatorStatus) | ||
| require.NoError(t, err) | ||
| configClient, err := configclient.NewForConfig(kubeConfig) | ||
| require.NoError(t, err) | ||
|
|
||
| ctx := context.Background() | ||
| _, operatorStatus, _, err := operatorClient.GetStaticPodOperatorStateWithQuorum(ctx) | ||
| require.NoError(t, err) | ||
| require.True(t, v1helpers.IsOperatorConditionTrue(operatorStatus.Conditions, "CertRotationTimeUpgradeable")) | ||
|
|
||
| kubeClient := kubernetes.NewForConfigOrDie(kubeConfig) | ||
| t.Logf("Creating unsupported-cert-rotation-config...") | ||
| _, err = kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).Create(context.TODO(), &corev1.ConfigMap{ | ||
| ObjectMeta: metav1.ObjectMeta{Namespace: operatorclient.GlobalUserSpecifiedConfigNamespace, Name: "unsupported-cert-rotation-config"}, | ||
| Data: map[string]string{"base": "2y"}, | ||
| }, metav1.CreateOptions{}) | ||
| require.NoError(t, err) | ||
| defer func() { | ||
| kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).Delete(context.TODO(), "unsupported-cert-rotation-config", metav1.DeleteOptions{}) | ||
| }() | ||
|
|
||
| err = wait.PollImmediate(1*time.Second, 5*time.Second, func() (bool, error) { | ||
| _, operatorStatus, _, err := operatorClient.GetStaticPodOperatorStateWithQuorum(ctx) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| clusteroperator, err := configClient.ClusterOperators().Get(context.TODO(), "kube-apiserver", metav1.GetOptions{}) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| certRotationCondition := v1helpers.FindOperatorCondition(operatorStatus.Conditions, "CertRotationTimeUpgradeable") | ||
| upgradeableCondition := configv1helpers.FindStatusCondition(clusteroperator.Status.Conditions, "Upgradeable") | ||
| if certRotationCondition == nil || upgradeableCondition == nil { | ||
| return false, fmt.Errorf("Couldn't find CertRotationTimeUpgradeable or Upgradeable condition") | ||
| } | ||
| if certRotationCondition.Status == operatorv1.ConditionFalse && | ||
| upgradeableCondition.Status == configv1.ConditionFalse && strings.Contains(upgradeableCondition.Reason, "CertRotationTime") { | ||
| return true, nil | ||
| } | ||
| t.Logf("\nCertRotationTimeUpgradeable: %#v\nUpgradeable: %#v", certRotationCondition, upgradeableCondition) | ||
| return false, nil | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| t.Logf("Removing unsupported-cert-rotation-config...") | ||
| err = kubeClient.CoreV1().ConfigMaps(operatorclient.GlobalUserSpecifiedConfigNamespace).Delete(context.TODO(), "unsupported-cert-rotation-config", metav1.DeleteOptions{}) | ||
| require.NoError(t, err) | ||
|
|
||
| err = wait.PollImmediate(1*time.Second, 5*time.Second, func() (bool, error) { | ||
| _, operatorStatus, _, err := operatorClient.GetStaticPodOperatorStateWithQuorum(ctx) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| clusteroperator, err := configClient.ClusterOperators().Get(context.TODO(), "kube-apiserver", metav1.GetOptions{}) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| certRotationCondition := v1helpers.FindOperatorCondition(operatorStatus.Conditions, "CertRotationTimeUpgradeable") | ||
| upgradeableCondition := configv1helpers.FindStatusCondition(clusteroperator.Status.Conditions, "Upgradeable") | ||
| if certRotationCondition == nil || upgradeableCondition == nil { | ||
| return false, fmt.Errorf("Couldn't find CertRotationTimeUpgradeable or Upgradeable condition") | ||
| } | ||
| if certRotationCondition.Status == operatorv1.ConditionTrue && | ||
| (upgradeableCondition.Status == configv1.ConditionTrue || !strings.Contains(upgradeableCondition.Reason, "CertRotationTime")) { | ||
| return true, nil | ||
| } | ||
| t.Logf("\nCertRotationTimeUpgradeable: %#v\nUpgradeable: %#v", certRotationCondition, upgradeableCondition) | ||
| return false, nil | ||
| }) | ||
| require.NoError(t, err) | ||
| testCertRotationTimeUpgradeable(t) | ||
| } | ||
|
|
||
| // This test calls the shared function which | ||
| // can be called from both standard Go tests and Ginkgo | ||
| // | ||
| // This situation is temporary until we test the new e2e-gcp-operator-serial-ote job. | ||
| // Eventually all tests will be run only as part of the OTE framework. | ||
| func TestCertRotationStompOnBadType(t *testing.T) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
| kubeConfig, err := test.NewClientConfigForTest() | ||
| require.NoError(t, err) | ||
| kubeClient := kubernetes.NewForConfigOrDie(kubeConfig) | ||
|
|
||
| // this is inherently racy against a controller | ||
| err = wait.PollImmediate(10*time.Millisecond, 5*time.Second, func() (done bool, err error) { | ||
| if err := kubeClient.CoreV1().Secrets(operatorclient.OperatorNamespace).Delete(context.TODO(), "aggregator-client-signer", metav1.DeleteOptions{}); err != nil { | ||
| return false, nil | ||
| } | ||
| if _, err := kubeClient.CoreV1().Secrets(operatorclient.OperatorNamespace).Create(context.TODO(), &corev1.Secret{ | ||
| ObjectMeta: metav1.ObjectMeta{Namespace: operatorclient.OperatorNamespace, Name: "aggregator-client-signer"}, | ||
| Type: "SecretTypeTLS", | ||
| }, metav1.CreateOptions{}); err != nil { | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| err = wait.PollImmediate(100*time.Millisecond, 30*time.Second, func() (done bool, err error) { | ||
| curr, err := kubeClient.CoreV1().Secrets(operatorclient.OperatorNamespace).Get(context.TODO(), "aggregator-client-signer", metav1.GetOptions{}) | ||
| if errors.IsNotFound(err) { | ||
| return false, nil | ||
| } | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| if curr.Type == corev1.SecretTypeTLS { | ||
| return true, nil | ||
| } | ||
| return false, nil | ||
| }) | ||
| require.NoError(t, err) | ||
| testCertRotationStompOnBadType(t) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you adda a new tmp commit with t.Fail() just to see if the expected CI jobs will fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other than that LGTM