From a55e9de4fc70bad9785d12e4b18fd59ce8292db3 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Wed, 23 Sep 2020 07:50:45 -0400 Subject: [PATCH] Use existing clock test double to get kubecertagent units passing Signed-off-by: Andrew Keesler --- internal/controller/kubecertagent/execer.go | 26 ++++---------- .../controller/kubecertagent/execer_test.go | 36 +++++++------------ 2 files changed, 18 insertions(+), 44 deletions(-) diff --git a/internal/controller/kubecertagent/execer.go b/internal/controller/kubecertagent/execer.go index 2435c474..7993fbd6 100644 --- a/internal/controller/kubecertagent/execer.go +++ b/internal/controller/kubecertagent/execer.go @@ -9,6 +9,7 @@ import ( v1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/clock" corev1informers "k8s.io/client-go/informers/core/v1" "k8s.io/klog/v2" @@ -21,28 +22,13 @@ import ( "go.pinniped.dev/internal/provider" ) -type CurrentTimeProvider interface { - Now() metav1.Time -} - -type currentTimeProvider struct{} - -// TODO use this as the currentTimeProvider argument when calling NewExecerController() from prepare_controllers.go. -func NewCurrentTimeProvider() CurrentTimeProvider { - return ¤tTimeProvider{} -} - -func (f *currentTimeProvider) Now() metav1.Time { - return metav1.Now() -} - type execerController struct { agentInfo *Info credentialIssuerConfigNamespaceName string credentialIssuerConfigResourceName string dynamicCertProvider provider.DynamicTLSServingCertProvider podCommandExecutor kubecertauthority.PodCommandExecutor - currentTimeProvider CurrentTimeProvider + clock clock.Clock pinnipedAPIClient pinnipedclientset.Interface agentPodInformer corev1informers.PodInformer } @@ -54,7 +40,7 @@ func NewExecerController( dynamicCertProvider provider.DynamicTLSServingCertProvider, podCommandExecutor kubecertauthority.PodCommandExecutor, pinnipedAPIClient pinnipedclientset.Interface, - currentTimeProvider CurrentTimeProvider, + clock clock.Clock, agentPodInformer corev1informers.PodInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, ) controllerlib.Controller { @@ -67,8 +53,8 @@ func NewExecerController( credentialIssuerConfigResourceName: credentialIssuerConfigResourceName, dynamicCertProvider: dynamicCertProvider, podCommandExecutor: podCommandExecutor, - currentTimeProvider: currentTimeProvider, pinnipedAPIClient: pinnipedAPIClient, + clock: clock, agentPodInformer: agentPodInformer, }, }, @@ -145,7 +131,7 @@ func (c *execerController) strategySuccess() configv1alpha1.CredentialIssuerConf Status: configv1alpha1.SuccessStrategyStatus, Reason: configv1alpha1.FetchedKeyStrategyReason, Message: "Key was fetched successfully", - LastUpdateTime: c.currentTimeProvider.Now(), + LastUpdateTime: metav1.NewTime(c.clock.Now()), } } @@ -155,7 +141,7 @@ func (c *execerController) strategyError(err error) configv1alpha1.CredentialIss Status: configv1alpha1.ErrorStrategyStatus, Reason: configv1alpha1.CouldNotFetchKeyStrategyReason, Message: err.Error(), - LastUpdateTime: c.currentTimeProvider.Now(), + LastUpdateTime: metav1.NewTime(c.clock.Now()), } } diff --git a/internal/controller/kubecertagent/execer_test.go b/internal/controller/kubecertagent/execer_test.go index c5271c28..3c36c3c8 100644 --- a/internal/controller/kubecertagent/execer_test.go +++ b/internal/controller/kubecertagent/execer_test.go @@ -16,6 +16,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/clock" kubeinformers "k8s.io/client-go/informers" kubernetesfake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" @@ -27,18 +28,6 @@ import ( "go.pinniped.dev/internal/testutil" ) -type fakeCurrentTimeProvider struct { - frozenNow *metav1.Time -} - -func (f *fakeCurrentTimeProvider) Now() metav1.Time { - if f.frozenNow == nil { - realNow := metav1.Now() - f.frozenNow = &realNow - } - return *f.frozenNow -} - func TestExecerControllerOptions(t *testing.T) { spec.Run(t, "options", func(t *testing.T, when spec.G, it spec.S) { var r *require.Assertions @@ -68,10 +57,10 @@ func TestExecerControllerOptions(t *testing.T) { }, "credentialIssuerConfigNamespaceName", "credentialIssuerConfigResourceName", - nil, // not needed for this test - nil, // not needed for this test - nil, // not needed for this test - &fakeCurrentTimeProvider{}, + nil, // dynamicCertProvider, not needed for this test + nil, // podCommandExecutor, not needed for this test + nil, // pinnipedAPIClient, not needed for this test + nil, // clock, not needed for this test agentPodsInformer, observableWithInformerOption.WithInformer, ) @@ -174,8 +163,8 @@ func TestManagerControllerSync(t *testing.T) { var agentPodTemplate *corev1.Pod var dynamicCertProvider provider.DynamicTLSServingCertProvider var fakeCertPEM, fakeKeyPEM string - var fakeNow *fakeCurrentTimeProvider var credentialIssuerConfigGVR schema.GroupVersionResource + var frozenNow time.Time // Defer starting the informers until the last possible moment so that the // nested Before's can keep adding things to the informer caches. @@ -192,7 +181,7 @@ func TestManagerControllerSync(t *testing.T) { dynamicCertProvider, fakeExecutor, pinnipedAPIClient, - fakeNow, + clock.NewFakeClock(frozenNow), agentPodInformer.Core().V1().Pods(), controllerlib.WithInformer, ) @@ -251,8 +240,7 @@ func TestManagerControllerSync(t *testing.T) { agentPodInformerClient = kubernetesfake.NewSimpleClientset() agentPodInformer = kubeinformers.NewSharedInformerFactory(agentPodInformerClient, 0) fakeExecutor = &fakePodExecutor{r: r} - fakeNow = &fakeCurrentTimeProvider{} - fakeNow.Now() // call once to initialize + frozenNow = time.Date(2020, time.September, 23, 7, 42, 0, 0, time.Local) dynamicCertProvider = provider.NewDynamicTLSServingCertProvider() dynamicCertProvider.Set([]byte(defaultDynamicCertProviderCert), []byte(defaultDynamicCertProviderKey)) @@ -402,7 +390,7 @@ func TestManagerControllerSync(t *testing.T) { Status: configv1alpha1.SuccessStrategyStatus, Reason: configv1alpha1.FetchedKeyStrategyReason, Message: "Key was fetched successfully", - LastUpdateTime: fakeNow.Now(), + LastUpdateTime: metav1.NewTime(frozenNow), }, } expectedGetAction := coretesting.NewGetAction(credentialIssuerConfigGVR, credentialIssuerConfigNamespaceName, credentialIssuerConfigResourceName) @@ -432,7 +420,7 @@ func TestManagerControllerSync(t *testing.T) { Status: configv1alpha1.SuccessStrategyStatus, Reason: configv1alpha1.FetchedKeyStrategyReason, Message: "Key was fetched successfully", - LastUpdateTime: fakeNow.Now(), + LastUpdateTime: metav1.NewTime(frozenNow), }, }, }, @@ -475,7 +463,7 @@ func TestManagerControllerSync(t *testing.T) { Status: configv1alpha1.ErrorStrategyStatus, Reason: configv1alpha1.CouldNotFetchKeyStrategyReason, Message: podExecErrorMessage, - LastUpdateTime: metav1.Now(), + LastUpdateTime: metav1.NewTime(frozenNow), }, }, }, @@ -517,7 +505,7 @@ func TestManagerControllerSync(t *testing.T) { Status: configv1alpha1.ErrorStrategyStatus, Reason: configv1alpha1.CouldNotFetchKeyStrategyReason, Message: podExecErrorMessage, - LastUpdateTime: metav1.Now(), + LastUpdateTime: metav1.NewTime(frozenNow), }, }, },