Skip to content

Commit 3883e25

Browse files
committed
Harmonize reconciler logic and improve logging
1 parent 77b672d commit 3883e25

File tree

10 files changed

+187
-204
lines changed

10 files changed

+187
-204
lines changed

.github/workflows/lint.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,4 @@ jobs:
1717
- name: golangci-lint
1818
uses: golangci/golangci-lint-action@v9
1919
with:
20-
version: v2.5
20+
version: v2.6

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ CONTROLLER_TOOLS_VERSION ?= v0.17.1
201201
ENVTEST_VERSION ?= $(shell go list -m -f "{{ .Version }}" sigs.k8s.io/controller-runtime | awk -F'[v.]' '{printf "release-%d.%d", $$2, $$3}')
202202
#ENVTEST_K8S_VERSION is the version of Kubernetes to use for setting up ENVTEST binaries (i.e. 1.31)
203203
ENVTEST_K8S_VERSION ?= $(shell go list -m -f "{{ .Version }}" k8s.io/api | awk -F'[v.]' '{printf "1.%d.%d",$$3, $$2}')
204-
GOLANGCI_LINT_VERSION ?= v2.1
204+
GOLANGCI_LINT_VERSION ?= v2.6
205205
ADDLICENSE_VERSION ?= v1.1.1
206206
GOIMPORTS_VERSION ?= v0.31.0
207207
GEN_CRD_API_REFERENCE_DOCS_VERSION ?= v0.3.0

cmdutils/suite_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ var _ = BeforeSuite(func() {
5959
// Note that you must have the required binaries setup under the bin directory to perform
6060
// the tests directly. When we run make test it will be setup and used automatically.
6161
BinaryAssetsDirectory: filepath.Join("..", "bin", "k8s",
62-
fmt.Sprintf("1.31.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
62+
fmt.Sprintf("1.34.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
6363
}
6464

6565
sourceCfg, err := sourceEnv.Start()
@@ -89,7 +89,7 @@ var _ = BeforeSuite(func() {
8989
// Note that you must have the required binaries setup under the bin directory to perform
9090
// the tests directly. When we run make test it will be setup and used automatically.
9191
BinaryAssetsDirectory: filepath.Join("..", "bin", "k8s",
92-
fmt.Sprintf("1.31.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
92+
fmt.Sprintf("1.34.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
9393
}
9494

9595
// cfg is defined in this file globally.

docs/api-reference/api.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ string
4848
<td>
4949
<code>metadata</code><br/>
5050
<em>
51-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#objectmeta-v1-meta">
51+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#objectmeta-v1-meta">
5252
Kubernetes meta/v1.ObjectMeta
5353
</a>
5454
</em>
@@ -86,7 +86,7 @@ string
8686
<td>
8787
<code>ignitionSecretRef</code><br/>
8888
<em>
89-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#localobjectreference-v1-core">
89+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#localobjectreference-v1-core">
9090
Kubernetes core/v1.LocalObjectReference
9191
</a>
9292
</em>
@@ -168,7 +168,7 @@ string
168168
<td>
169169
<code>metadata</code><br/>
170170
<em>
171-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#objectmeta-v1-meta">
171+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#objectmeta-v1-meta">
172172
Kubernetes meta/v1.ObjectMeta
173173
</a>
174174
</em>
@@ -272,7 +272,7 @@ string
272272
<td>
273273
<code>ignitionSecretRef</code><br/>
274274
<em>
275-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#localobjectreference-v1-core">
275+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#localobjectreference-v1-core">
276276
Kubernetes core/v1.LocalObjectReference
277277
</a>
278278
</em>
@@ -285,7 +285,7 @@ Kubernetes core/v1.LocalObjectReference
285285
<td>
286286
<code>ipxeScriptSecretRef</code><br/>
287287
<em>
288-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#localobjectreference-v1-core">
288+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#localobjectreference-v1-core">
289289
Kubernetes core/v1.LocalObjectReference
290290
</a>
291291
</em>
@@ -342,7 +342,7 @@ string
342342
<td>
343343
<code>ignitionSecretRef</code><br/>
344344
<em>
345-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#localobjectreference-v1-core">
345+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#localobjectreference-v1-core">
346346
Kubernetes core/v1.LocalObjectReference
347347
</a>
348348
</em>
@@ -432,7 +432,7 @@ HTTPBootConfigState
432432
<td>
433433
<code>conditions</code><br/>
434434
<em>
435-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta">
435+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#condition-v1-meta">
436436
[]Kubernetes meta/v1.Condition
437437
</a>
438438
</em>
@@ -540,7 +540,7 @@ string
540540
<td>
541541
<code>ignitionSecretRef</code><br/>
542542
<em>
543-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#localobjectreference-v1-core">
543+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#localobjectreference-v1-core">
544544
Kubernetes core/v1.LocalObjectReference
545545
</a>
546546
</em>
@@ -553,7 +553,7 @@ Kubernetes core/v1.LocalObjectReference
553553
<td>
554554
<code>ipxeScriptSecretRef</code><br/>
555555
<em>
556-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#localobjectreference-v1-core">
556+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#localobjectreference-v1-core">
557557
Kubernetes core/v1.LocalObjectReference
558558
</a>
559559
</em>
@@ -622,7 +622,7 @@ IPXEBootConfigState
622622
<td>
623623
<code>conditions</code><br/>
624624
<em>
625-
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta">
625+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#condition-v1-meta">
626626
[]Kubernetes meta/v1.Condition
627627
</a>
628628
</em>

hack/api-reference/config.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
},
2222
{
2323
"typeMatchPrefix": "^k8s\\.io/(api|apimachinery/pkg/apis)/",
24-
"docsURLTemplate": "https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#{{lower .TypeIdentifier}}-{{arrIndex .PackageSegments -1}}-{{arrIndex .PackageSegments -2}}"
24+
"docsURLTemplate": "https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.34/#{{lower .TypeIdentifier}}-{{arrIndex .PackageSegments -1}}-{{arrIndex .PackageSegments -2}}"
2525
}
2626
],
2727
"typeDisplayNamePrefixOverrides": {

internal/controller/httpbootconfig_controller.go

Lines changed: 54 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
ctrl "sigs.k8s.io/controller-runtime"
1414
"sigs.k8s.io/controller-runtime/pkg/client"
1515
"sigs.k8s.io/controller-runtime/pkg/handler"
16-
"sigs.k8s.io/controller-runtime/pkg/log"
1716
"sigs.k8s.io/controller-runtime/pkg/reconcile"
1817

1918
"github.com/go-logr/logr"
@@ -32,55 +31,53 @@ type HTTPBootConfigReconciler struct {
3231
//+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch
3332

3433
func (r *HTTPBootConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
35-
log := log.FromContext(ctx)
36-
37-
HTTPBootConfig := &bootv1alpha1.HTTPBootConfig{}
38-
if err := r.Get(ctx, req.NamespacedName, HTTPBootConfig); err != nil {
34+
log := ctrl.LoggerFrom(ctx)
35+
config := &bootv1alpha1.HTTPBootConfig{}
36+
if err := r.Get(ctx, req.NamespacedName, config); err != nil {
3937
return ctrl.Result{}, client.IgnoreNotFound(err)
4038
}
41-
42-
return r.reconcileExists(ctx, log, HTTPBootConfig)
39+
return r.reconcileExists(ctx, log, config)
4340
}
4441

45-
func (r *HTTPBootConfigReconciler) reconcileExists(ctx context.Context, log logr.Logger, HTTPBootConfig *bootv1alpha1.HTTPBootConfig) (ctrl.Result, error) {
46-
if !HTTPBootConfig.DeletionTimestamp.IsZero() {
47-
return r.delete(ctx, log, HTTPBootConfig)
42+
func (r *HTTPBootConfigReconciler) reconcileExists(ctx context.Context, log logr.Logger, config *bootv1alpha1.HTTPBootConfig) (ctrl.Result, error) {
43+
if !config.DeletionTimestamp.IsZero() {
44+
return r.delete(ctx, log, config)
4845
}
49-
50-
return r.reconcile(ctx, log, HTTPBootConfig)
46+
return r.reconcile(ctx, log, config)
5147
}
5248

53-
func (r *HTTPBootConfigReconciler) reconcile(ctx context.Context, log logr.Logger, HTTPBootConfig *bootv1alpha1.HTTPBootConfig) (ctrl.Result, error) {
49+
func (r *HTTPBootConfigReconciler) reconcile(ctx context.Context, log logr.Logger, config *bootv1alpha1.HTTPBootConfig) (ctrl.Result, error) {
50+
log.V(1).Info("Reconciling HTTPBootConfig")
51+
5452
log.V(1).Info("Ensuring Ignition")
55-
state, ignitionErr := r.ensureIgnition(ctx, log, HTTPBootConfig)
56-
if ignitionErr != nil {
57-
patchError := r.patchStatus(ctx, HTTPBootConfig, state)
58-
if patchError != nil {
59-
return ctrl.Result{}, fmt.Errorf("failed to patch status %w %w", ignitionErr, patchError)
53+
state, err := r.ensureIgnition(ctx, log, config)
54+
if err != nil {
55+
if err := r.patchStatus(ctx, config, state); err != nil {
56+
return ctrl.Result{}, err
6057
}
61-
62-
log.V(1).Info("Failed to Ensure Ignition", "Error", ignitionErr)
58+
log.V(1).Info("Failed to Ensure Ignition", "Error", err)
6359
return ctrl.Result{}, nil
6460
}
61+
log.V(1).Info("Ensured Ignition")
6562

66-
patchErr := r.patchStatus(ctx, HTTPBootConfig, state)
67-
if patchErr != nil {
68-
return ctrl.Result{}, fmt.Errorf("failed to patch status %w", patchErr)
63+
if err := r.patchStatus(ctx, config, state); err != nil {
64+
return ctrl.Result{}, err
6965
}
7066

67+
log.V(1).Info("Reconciled HTTPBootConfig")
7168
return ctrl.Result{}, nil
7269
}
7370

74-
func (r *HTTPBootConfigReconciler) ensureIgnition(ctx context.Context, _ logr.Logger, HTTPBootConfig *bootv1alpha1.HTTPBootConfig) (bootv1alpha1.HTTPBootConfigState, error) {
71+
func (r *HTTPBootConfigReconciler) ensureIgnition(ctx context.Context, _ logr.Logger, config *bootv1alpha1.HTTPBootConfig) (bootv1alpha1.HTTPBootConfigState, error) {
7572
// Verify if the IgnitionRef is set, and it has the intended data key.
76-
if HTTPBootConfig.Spec.IgnitionSecretRef != nil {
77-
IgnitionSecret := &corev1.Secret{}
78-
if err := r.Get(ctx, client.ObjectKey{Name: HTTPBootConfig.Spec.IgnitionSecretRef.Name, Namespace: HTTPBootConfig.Namespace}, IgnitionSecret); err != nil {
73+
if config.Spec.IgnitionSecretRef != nil {
74+
ignitionSecret := &corev1.Secret{}
75+
if err := r.Get(ctx, client.ObjectKey{Name: config.Spec.IgnitionSecretRef.Name, Namespace: config.Namespace}, ignitionSecret); err != nil {
7976
return bootv1alpha1.HTTPBootConfigStateError, err
80-
// TODO: Add some validation steps to ensure that the IgntionData is compliant, if necessary.
77+
// TODO: Add some validation steps to ensure that the IgnitionData is compliant, if necessary.
8178
// Assume for now, that it's going to json format.
8279
}
83-
if IgnitionSecret.Data[bootv1alpha1.DefaultIgnitionKey] == nil {
80+
if ignitionSecret.Data[bootv1alpha1.DefaultIgnitionKey] == nil {
8481
return bootv1alpha1.HTTPBootConfigStateError, fmt.Errorf("ignition data is missing")
8582
}
8683
}
@@ -93,64 +90,61 @@ func (r *HTTPBootConfigReconciler) delete(_ context.Context, log logr.Logger, _
9390

9491
// TODO
9592

93+
log.V(1).Info("Deleted HTTPBootConfig")
9694
return ctrl.Result{}, nil
9795
}
9896

99-
// SetupWithManager sets up the controller with the Manager.
100-
func (r *HTTPBootConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
101-
return ctrl.NewControllerManagedBy(mgr).
102-
For(&bootv1alpha1.HTTPBootConfig{}).
103-
Watches(
104-
&corev1.Secret{},
105-
handler.EnqueueRequestsFromMapFunc(r.enqueueHTTPBootConfigReferencingIgnitionSecret),
106-
).
107-
Complete(r)
108-
}
109-
110-
func (r *HTTPBootConfigReconciler) patchStatus(
111-
ctx context.Context,
112-
HTTPBootConfig *bootv1alpha1.HTTPBootConfig,
113-
state bootv1alpha1.HTTPBootConfigState,
114-
) error {
115-
if HTTPBootConfig.Status.State == state {
97+
func (r *HTTPBootConfigReconciler) patchStatus(ctx context.Context, config *bootv1alpha1.HTTPBootConfig, state bootv1alpha1.HTTPBootConfigState) error {
98+
if config.Status.State == state {
11699
return nil
117100
}
118101

119-
base := HTTPBootConfig.DeepCopy()
120-
HTTPBootConfig.Status.State = state
102+
base := config.DeepCopy()
103+
config.Status.State = state
121104

122-
if err := r.Status().Patch(ctx, HTTPBootConfig, client.MergeFrom(base)); err != nil {
123-
return fmt.Errorf("error patching HTTPBootConfig: %w", err)
105+
if err := r.Status().Patch(ctx, config, client.MergeFrom(base)); err != nil {
106+
return err
124107
}
125108
return nil
126109
}
127110

128111
func (r *HTTPBootConfigReconciler) enqueueHTTPBootConfigReferencingIgnitionSecret(ctx context.Context, secret client.Object) []reconcile.Request {
129-
log := log.Log.WithValues("secret", secret.GetName())
112+
log := ctrl.LoggerFrom(ctx)
130113
secretObj, ok := secret.(*corev1.Secret)
131114
if !ok {
132115
log.Error(nil, "cant decode object into Secret", secret)
133116
return nil
134117
}
135118

136-
list := &bootv1alpha1.HTTPBootConfigList{}
137-
if err := r.List(ctx, list, client.InNamespace("")); err != nil {
138-
log.Error(err, "failed to list HTTPBootConfig for secret", secret)
119+
configList := &bootv1alpha1.HTTPBootConfigList{}
120+
if err := r.List(ctx, configList, client.InNamespace("")); err != nil {
121+
log.Error(err, "failed to list HTTPBootConfig for Secret", "Secret", client.ObjectKeyFromObject(secretObj))
139122
return nil
140123
}
141124

142125
var requests []reconcile.Request
143-
for _, HTTPBootConfig := range list.Items {
144-
if HTTPBootConfig.Spec.IgnitionSecretRef != nil &&
145-
HTTPBootConfig.Spec.IgnitionSecretRef.Name == secretObj.Name &&
146-
HTTPBootConfig.Namespace == secretObj.Namespace {
126+
for _, config := range configList.Items {
127+
if config.Spec.IgnitionSecretRef != nil &&
128+
config.Spec.IgnitionSecretRef.Name == secretObj.Name &&
129+
config.Namespace == secretObj.Namespace {
147130
requests = append(requests, reconcile.Request{
148131
NamespacedName: types.NamespacedName{
149-
Name: HTTPBootConfig.Name,
150-
Namespace: HTTPBootConfig.Namespace,
132+
Name: config.Name,
133+
Namespace: config.Namespace,
151134
},
152135
})
153136
}
154137
}
155138
return requests
156139
}
140+
141+
// SetupWithManager sets up the controller with the Manager.
142+
func (r *HTTPBootConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
143+
return ctrl.NewControllerManagedBy(mgr).
144+
For(&bootv1alpha1.HTTPBootConfig{}).
145+
Watches(
146+
&corev1.Secret{},
147+
handler.EnqueueRequestsFromMapFunc(r.enqueueHTTPBootConfigReferencingIgnitionSecret),
148+
).
149+
Complete(r)
150+
}

0 commit comments

Comments
 (0)