Skip to content

Commit ea06d84

Browse files
irbyirby
committed
fix: resolve issue with default health check interval override
Signed-off-by: Matthew H. Irby <[email protected]>
1 parent dace4df commit ea06d84

File tree

2 files changed

+66
-61
lines changed

2 files changed

+66
-61
lines changed

‎internal/controller/issuer_controller.go‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,10 @@ const (
3939
)
4040

4141
var (
42-
errGetAuthSecret = errors.New("failed to get Secret containing Issuer credentials")
43-
errGetCaSecret = errors.New("caSecretName specified a name, but failed to get Secret containing CA certificate")
44-
errHealthCheckerBuilder = errors.New("failed to build the healthchecker")
45-
errHealthCheckerCheck = errors.New("healthcheck failed")
46-
defaultHealthCheckInterval = time.Minute
42+
errGetAuthSecret = errors.New("failed to get Secret containing Issuer credentials")
43+
errGetCaSecret = errors.New("caSecretName specified a name, but failed to get Secret containing CA certificate")
44+
errHealthCheckerBuilder = errors.New("failed to build the healthchecker")
45+
errHealthCheckerCheck = errors.New("healthcheck failed")
4746
)
4847

4948
// IssuerReconciler reconciles a Issuer object
@@ -68,7 +67,6 @@ func (r *IssuerReconciler) newIssuer() (commandissuer.IssuerLike, error) {
6867
if err != nil {
6968
return nil, err
7069
}
71-
defaultHealthCheckInterval = r.DefaultHealthCheckInterval
7270
return ro.(commandissuer.IssuerLike), nil
7371
}
7472

@@ -100,7 +98,7 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
10098
}
10199
}()
102100

103-
healthCheckInterval, err := getHealthCheckInterval(log, issuer)
101+
healthCheckInterval, err := r.getHealthCheckInterval(log, issuer)
104102
if err != nil {
105103
log.Error(err, "an error occurred while getting the health check interval")
106104
issuer.GetStatus().SetCondition(ctx, commandissuer.IssuerConditionReady, commandissuer.ConditionFalse, issuerReadyConditionReason, err.Error())
@@ -157,14 +155,14 @@ func (r *IssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
157155
return ctrl.Result{RequeueAfter: healthCheckInterval}, nil
158156
}
159157

160-
func getHealthCheckInterval(log logr.Logger, issuer commandissuer.IssuerLike) (time.Duration, error) {
158+
func (r *IssuerReconciler) getHealthCheckInterval(log logr.Logger, issuer commandissuer.IssuerLike) (time.Duration, error) {
161159
spec := issuer.GetSpec()
162160

163-
defaultInterval := int(defaultHealthCheckInterval / time.Second)
161+
defaultInterval := int(r.DefaultHealthCheckInterval / time.Second)
164162

165163
if spec.HealthCheck == nil {
166-
log.Info(fmt.Sprintf("health check spec value is nil, using default: %d", defaultInterval))
167-
return defaultHealthCheckInterval, nil
164+
log.Info(fmt.Sprintf("health check spec value is nil, using default: %d seconds", defaultInterval))
165+
return r.DefaultHealthCheckInterval, nil
168166
}
169167

170168
if !spec.HealthCheck.Enabled {
@@ -173,8 +171,8 @@ func getHealthCheckInterval(log logr.Logger, issuer commandissuer.IssuerLike) (t
173171
}
174172

175173
if spec.HealthCheck.Interval == nil {
176-
log.Info(fmt.Sprintf("health check spec value is nil, using default: %d", defaultInterval))
177-
return defaultHealthCheckInterval, nil
174+
log.Info(fmt.Sprintf("health check spec value is nil, using default: %d seconds", defaultInterval))
175+
return r.DefaultHealthCheckInterval, nil
178176
}
179177

180178
healthCheckInterval := *spec.HealthCheck.Interval

‎internal/controller/issuer_controller_test.go‎

Lines changed: 55 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ func TestIssuerReconcile(t *testing.T) {
6767
objects []client.Object
6868
healthCheckerBuilder command.HealthCheckerBuilder
6969
clusterResourceNamespace string
70+
defaultHealthCheckInterval *time.Duration
7071
expectedResult ctrl.Result
7172
expectedError error
7273
expectedReadyConditionStatus commandissuerv1alpha1.ConditionStatus
@@ -114,7 +115,7 @@ func TestIssuerReconcile(t *testing.T) {
114115
healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false),
115116
expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue,
116117
expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse,
117-
expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval},
118+
expectedResult: ctrl.Result{RequeueAfter: time.Minute},
118119
},
119120
"issuer-basicauth-no-username": {
120121
kind: "Issuer",
@@ -236,7 +237,7 @@ func TestIssuerReconcile(t *testing.T) {
236237
clusterResourceNamespace: "kube-system",
237238
expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue,
238239
expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse,
239-
expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval},
240+
expectedResult: ctrl.Result{RequeueAfter: time.Minute},
240241
},
241242
"success-issuer-oauth": {
242243
kind: "Issuer",
@@ -277,7 +278,7 @@ func TestIssuerReconcile(t *testing.T) {
277278
healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false),
278279
expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue,
279280
expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse,
280-
expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval},
281+
expectedResult: ctrl.Result{RequeueAfter: time.Minute},
281282
},
282283
"issuer-oauth-no-tokenurl": {
283284
kind: "Issuer",
@@ -447,7 +448,7 @@ func TestIssuerReconcile(t *testing.T) {
447448
clusterResourceNamespace: "kube-system",
448449
expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue,
449450
expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionFalse,
450-
expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval},
451+
expectedResult: ctrl.Result{RequeueAfter: time.Minute},
451452
},
452453
"issuer-kind-Unrecognized": {
453454
kind: "UnrecognizedType",
@@ -733,7 +734,7 @@ func TestIssuerReconcile(t *testing.T) {
733734
clusterResourceNamespace: "kube-system",
734735
expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue,
735736
expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue,
736-
expectedResult: ctrl.Result{RequeueAfter: defaultHealthCheckInterval},
737+
expectedResult: ctrl.Result{RequeueAfter: time.Minute},
737738
},
738739
"success-nil-healthcheck-interval-defaults": {
739740
kind: "ClusterIssuer",
@@ -777,6 +778,46 @@ func TestIssuerReconcile(t *testing.T) {
777778
expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue,
778779
expectedResult: ctrl.Result{RequeueAfter: time.Duration(60) * time.Second},
779780
},
781+
"success-default-healthcheck-interval": {
782+
kind: "ClusterIssuer",
783+
name: types.NamespacedName{Name: "clusterissuer1"},
784+
objects: []client.Object{
785+
&commandissuerv1alpha1.ClusterIssuer{
786+
ObjectMeta: metav1.ObjectMeta{
787+
Name: "clusterissuer1",
788+
},
789+
Spec: commandissuerv1alpha1.IssuerSpec{
790+
SecretName: "clusterissuer1-credentials",
791+
HealthCheck: nil,
792+
},
793+
Status: commandissuerv1alpha1.IssuerStatus{
794+
Conditions: []commandissuerv1alpha1.IssuerCondition{
795+
{
796+
Type: commandissuerv1alpha1.IssuerConditionReady,
797+
Status: commandissuerv1alpha1.ConditionUnknown,
798+
},
799+
},
800+
},
801+
},
802+
&corev1.Secret{
803+
Type: corev1.SecretTypeBasicAuth,
804+
ObjectMeta: metav1.ObjectMeta{
805+
Name: "clusterissuer1-credentials",
806+
Namespace: "kube-system",
807+
},
808+
Data: map[string][]byte{
809+
corev1.BasicAuthUsernameKey: []byte("username"),
810+
corev1.BasicAuthPasswordKey: []byte("password"),
811+
},
812+
},
813+
},
814+
defaultHealthCheckInterval: to.Ptr(time.Duration(2) * time.Minute),
815+
healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, true),
816+
clusterResourceNamespace: "kube-system",
817+
expectedReadyConditionStatus: commandissuerv1alpha1.ConditionTrue,
818+
expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue,
819+
expectedResult: ctrl.Result{RequeueAfter: time.Duration(2) * time.Minute},
820+
},
780821
"success-nil-healthcheck-defaults": {
781822
kind: "ClusterIssuer",
782823
name: types.NamespacedName{Name: "clusterissuer1"},
@@ -816,48 +857,6 @@ func TestIssuerReconcile(t *testing.T) {
816857
expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionTrue,
817858
expectedResult: ctrl.Result{RequeueAfter: time.Duration(60) * time.Second},
818859
},
819-
// "error-healthcheck-invalid-parsing": {
820-
// kind: "Issuer",
821-
// name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"},
822-
// objects: []client.Object{
823-
// &commandissuerv1alpha1.Issuer{
824-
// ObjectMeta: metav1.ObjectMeta{
825-
// Name: "issuer1",
826-
// Namespace: "ns1",
827-
// },
828-
// Spec: commandissuerv1alpha1.IssuerSpec{
829-
// SecretName: "issuer1-credentials",
830-
// HealthCheck: &commandissuerv1alpha1.HealthCheckConfig{
831-
// Enabled: true,
832-
// Interval: to.Ptr(metav1.Duration{Duration: 30 * time.Second}),
833-
// },
834-
// },
835-
// Status: commandissuerv1alpha1.IssuerStatus{
836-
// Conditions: []commandissuerv1alpha1.IssuerCondition{
837-
// {
838-
// Type: commandissuerv1alpha1.IssuerConditionReady,
839-
// Status: commandissuerv1alpha1.ConditionUnknown,
840-
// },
841-
// },
842-
// },
843-
// },
844-
// &corev1.Secret{
845-
// Type: corev1.SecretTypeBasicAuth,
846-
// ObjectMeta: metav1.ObjectMeta{
847-
// Name: "issuer1-credentials",
848-
// Namespace: "ns1",
849-
// },
850-
// Data: map[string][]byte{
851-
// corev1.BasicAuthUsernameKey: []byte("username"),
852-
// corev1.BasicAuthPasswordKey: []byte("password"),
853-
// },
854-
// },
855-
// },
856-
// healthCheckerBuilder: newFakeHealthCheckerBuilder(nil, nil, false),
857-
// expectedReadyConditionStatus: commandissuerv1alpha1.ConditionFalse,
858-
// expectedMetadataSupportedConditionStatus: commandissuerv1alpha1.ConditionUnknown,
859-
// expectedResult: ctrl.Result{},
860-
// },
861860
"error-healthcheck-minimum-value": {
862861
kind: "Issuer",
863862
name: types.NamespacedName{Namespace: "ns1", Name: "issuer1"},
@@ -916,15 +915,23 @@ func TestIssuerReconcile(t *testing.T) {
916915
if tc.kind == "" {
917916
tc.kind = "Issuer"
918917
}
918+
919+
defaultHealthcheckInterval := time.Minute
920+
921+
if tc.defaultHealthCheckInterval != nil {
922+
defaultHealthcheckInterval = *tc.defaultHealthCheckInterval
923+
}
924+
919925
controller := IssuerReconciler{
920926
Kind: tc.kind,
921927
Client: fakeClient,
922928
Scheme: scheme,
923929
HealthCheckerBuilder: tc.healthCheckerBuilder,
924930
ClusterResourceNamespace: tc.clusterResourceNamespace,
925931
SecretAccessGrantedAtClusterLevel: true,
926-
DefaultHealthCheckInterval: time.Minute,
932+
DefaultHealthCheckInterval: defaultHealthcheckInterval,
927933
}
934+
928935
result, err := controller.Reconcile(
929936
ctrl.LoggerInto(context.TODO(), logrtesting.NewTestLogger(t)),
930937
reconcile.Request{NamespacedName: tc.name},

0 commit comments

Comments
 (0)