Skip to content

Commit 4561b90

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #367 from kstrenkova/refactor-network-attachments
Refactor network attachments
2 parents 0551849 + d4c426f commit 4561b90

3 files changed

Lines changed: 142 additions & 134 deletions

File tree

‎internal/controller/common.go‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,11 @@ import (
1212
"crypto/sha256"
1313

1414
"github.com/go-logr/logr"
15+
networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
16+
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
1517
"github.com/openstack-k8s-operators/lib-common/modules/common/configmap"
1618
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
19+
nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment"
1720
"github.com/openstack-k8s-operators/lib-common/modules/common/pvc"
1821
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
1922
"gopkg.in/yaml.v3"
@@ -548,6 +551,101 @@ func GetCommonRbacRules(privileged bool) []rbacv1.PolicyRule {
548551
return []rbacv1.PolicyRule{rbacPolicyRule}
549552
}
550553

554+
// EnsureNetworkAttachments fetches NetworkAttachmentDefinitions and creates annotations
555+
func (r *Reconciler) EnsureNetworkAttachments(
556+
ctx context.Context,
557+
log logr.Logger,
558+
helper *helper.Helper,
559+
networkAttachments []string,
560+
namespace string,
561+
conditions *condition.Conditions,
562+
) (map[string]string, ctrl.Result, error) {
563+
nadList := []networkv1.NetworkAttachmentDefinition{}
564+
for _, netAtt := range networkAttachments {
565+
netAttachDef, err := nad.GetNADWithName(ctx, helper, netAtt, namespace)
566+
if err != nil {
567+
if k8s_errors.IsNotFound(err) {
568+
// Since the net-attach-def CR should have been manually created by the user and referenced in the spec,
569+
// we treat this as a warning because it means that the service will not be able to start.
570+
log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt))
571+
conditions.Set(condition.FalseCondition(
572+
condition.NetworkAttachmentsReadyCondition,
573+
condition.ErrorReason,
574+
condition.SeverityWarning,
575+
condition.NetworkAttachmentsReadyWaitingMessage,
576+
netAtt))
577+
return nil, ctrl.Result{RequeueAfter: time.Second * 10}, nil
578+
}
579+
conditions.Set(condition.FalseCondition(
580+
condition.NetworkAttachmentsReadyCondition,
581+
condition.ErrorReason,
582+
condition.SeverityWarning,
583+
condition.NetworkAttachmentsReadyErrorMessage,
584+
err.Error()))
585+
return nil, ctrl.Result{}, err
586+
}
587+
588+
if netAttachDef != nil {
589+
nadList = append(nadList, *netAttachDef)
590+
}
591+
}
592+
593+
serviceAnnotations, err := nad.EnsureNetworksAnnotation(nadList)
594+
if err != nil {
595+
return nil, ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w",
596+
networkAttachments, err)
597+
}
598+
599+
return serviceAnnotations, ctrl.Result{}, nil
600+
}
601+
602+
// VerifyNetworkAttachments verifies network status on the pod and updates conditions
603+
func (r *Reconciler) VerifyNetworkAttachments(
604+
ctx context.Context,
605+
helper *helper.Helper,
606+
instance client.Object,
607+
networkAttachments []string,
608+
serviceLabels map[string]string,
609+
nextWorkflowStep int,
610+
conditions *condition.Conditions,
611+
networkAttachmentStatus *map[string][]string,
612+
) (ctrl.Result, error) {
613+
if !r.PodExists(ctx, instance, nextWorkflowStep) {
614+
return ctrl.Result{}, nil
615+
}
616+
617+
networkReady, status, err := nad.VerifyNetworkStatusFromAnnotation(
618+
ctx,
619+
helper,
620+
networkAttachments,
621+
serviceLabels,
622+
1,
623+
)
624+
if err != nil {
625+
return ctrl.Result{}, err
626+
}
627+
628+
*networkAttachmentStatus = status
629+
630+
if networkReady {
631+
conditions.MarkTrue(
632+
condition.NetworkAttachmentsReadyCondition,
633+
condition.NetworkAttachmentsReadyMessage)
634+
} else {
635+
err := fmt.Errorf("%w: %s", ErrNetworkAttachmentsMismatch, networkAttachments)
636+
conditions.Set(condition.FalseCondition(
637+
condition.NetworkAttachmentsReadyCondition,
638+
condition.ErrorReason,
639+
condition.SeverityWarning,
640+
condition.NetworkAttachmentsReadyErrorMessage,
641+
err.Error()))
642+
643+
return ctrl.Result{}, err
644+
}
645+
646+
return ctrl.Result{}, nil
647+
}
648+
551649
// EnsureCloudsConfigMapExists ensures that frameworks like Tobiko and Horizon have password values
552650
// present in clouds.yaml. This code ensures that we set a default value of
553651
// 12345678 when password value is missing in the clouds.yaml

‎internal/controller/tempest_controller.go‎

Lines changed: 22 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,13 @@ import (
2020
"context"
2121
"fmt"
2222
"strconv"
23-
"time"
2423

2524
"github.com/go-logr/logr"
26-
networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
2725
"github.com/openstack-k8s-operators/lib-common/modules/common"
2826
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
2927
"github.com/openstack-k8s-operators/lib-common/modules/common/configmap"
3028
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
3129
"github.com/openstack-k8s-operators/lib-common/modules/common/labels"
32-
nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment"
3330
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
3431
testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
3532
"github.com/openstack-k8s-operators/test-operator/internal/tempest"
@@ -248,40 +245,16 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
248245
instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage)
249246
// Generate ConfigMaps - end
250247

251-
nadList := []networkv1.NetworkAttachmentDefinition{}
252-
for _, netAtt := range instance.Spec.NetworkAttachments {
253-
nad, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace)
254-
if err != nil {
255-
if k8s_errors.IsNotFound(err) {
256-
// Since the net-attach-def CR should have been manually created by the user and referenced in the spec,
257-
// we treat this as a warning because it means that the service will not be able to start.
258-
Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt))
259-
instance.Status.Conditions.Set(condition.FalseCondition(
260-
condition.NetworkAttachmentsReadyCondition,
261-
condition.ErrorReason,
262-
condition.SeverityWarning,
263-
condition.NetworkAttachmentsReadyWaitingMessage,
264-
netAtt))
265-
return ctrl.Result{RequeueAfter: time.Second * 10}, nil
266-
}
267-
instance.Status.Conditions.Set(condition.FalseCondition(
268-
condition.NetworkAttachmentsReadyCondition,
269-
condition.ErrorReason,
270-
condition.SeverityWarning,
271-
condition.NetworkAttachmentsReadyErrorMessage,
272-
err.Error()))
273-
return ctrl.Result{}, err
274-
}
275-
276-
if nad != nil {
277-
nadList = append(nadList, *nad)
278-
}
279-
}
280-
281-
serviceAnnotations, err := nad.EnsureNetworksAnnotation(nadList)
282-
if err != nil {
283-
return ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w",
284-
instance.Spec.NetworkAttachments, err)
248+
serviceAnnotations, ctrlResult, err := r.EnsureNetworkAttachments(
249+
ctx,
250+
Log,
251+
helper,
252+
instance.Spec.NetworkAttachments,
253+
instance.Namespace,
254+
&instance.Status.Conditions,
255+
)
256+
if err != nil || (ctrlResult != ctrl.Result{}) {
257+
return ctrlResult, err
285258
}
286259

287260
// Create a new pod
@@ -335,37 +308,19 @@ func (r *TempestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
335308
// Create a new pod - end
336309

337310
// NetworkAttachments
338-
if r.PodExists(ctx, instance, nextWorkflowStep) {
339-
networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(
340-
ctx,
341-
helper,
342-
instance.Spec.NetworkAttachments,
343-
serviceLabels,
344-
1,
345-
)
346-
if err != nil {
347-
return ctrl.Result{}, err
348-
}
349-
350-
instance.Status.NetworkAttachments = networkAttachmentStatus
351-
352-
if networkReady {
353-
instance.Status.Conditions.MarkTrue(
354-
condition.NetworkAttachmentsReadyCondition,
355-
condition.NetworkAttachmentsReadyMessage)
356-
} else {
357-
err := fmt.Errorf("%w: %s", ErrNetworkAttachmentsMismatch, instance.Spec.NetworkAttachments)
358-
instance.Status.Conditions.Set(condition.FalseCondition(
359-
condition.NetworkAttachmentsReadyCondition,
360-
condition.ErrorReason,
361-
condition.SeverityWarning,
362-
condition.NetworkAttachmentsReadyErrorMessage,
363-
err.Error()))
364-
365-
return ctrl.Result{}, err
366-
}
311+
ctrlResult, err = r.VerifyNetworkAttachments(
312+
ctx,
313+
helper,
314+
instance,
315+
instance.Spec.NetworkAttachments,
316+
serviceLabels,
317+
nextWorkflowStep,
318+
&instance.Status.Conditions,
319+
&instance.Status.NetworkAttachments,
320+
)
321+
if err != nil || (ctrlResult != ctrl.Result{}) {
322+
return ctrlResult, err
367323
}
368-
// NetworkAttachments - end
369324

370325
return ctrl.Result{}, nil
371326
}

‎internal/controller/tobiko_controller.go‎

Lines changed: 22 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,13 @@ import (
2020
"context"
2121
"fmt"
2222
"strconv"
23-
"time"
2423

2524
"github.com/go-logr/logr"
26-
networkv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
2725
"github.com/openstack-k8s-operators/lib-common/modules/common"
2826
"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
2927
"github.com/openstack-k8s-operators/lib-common/modules/common/configmap"
3028
"github.com/openstack-k8s-operators/lib-common/modules/common/env"
3129
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
32-
nad "github.com/openstack-k8s-operators/lib-common/modules/common/networkattachment"
3330
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
3431
testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
3532
"github.com/openstack-k8s-operators/test-operator/internal/tobiko"
@@ -221,74 +218,32 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
221218
}
222219
// Create PersistentVolumeClaim - end
223220

224-
nadList := []networkv1.NetworkAttachmentDefinition{}
225-
for _, netAtt := range instance.Spec.NetworkAttachments {
226-
nad, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace)
227-
if err != nil {
228-
if k8s_errors.IsNotFound(err) {
229-
// Since the net-attach-def CR should have been manually created by the user and referenced in the spec,
230-
// we treat this as a warning because it means that the service will not be able to start.
231-
Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt))
232-
instance.Status.Conditions.Set(condition.FalseCondition(
233-
condition.NetworkAttachmentsReadyCondition,
234-
condition.ErrorReason,
235-
condition.SeverityWarning,
236-
condition.NetworkAttachmentsReadyWaitingMessage,
237-
netAtt))
238-
return ctrl.Result{RequeueAfter: time.Second * 10}, nil
239-
}
240-
instance.Status.Conditions.Set(condition.FalseCondition(
241-
condition.NetworkAttachmentsReadyCondition,
242-
condition.ErrorReason,
243-
condition.SeverityWarning,
244-
condition.NetworkAttachmentsReadyErrorMessage,
245-
err.Error()))
246-
return ctrl.Result{}, err
247-
}
248-
249-
if nad != nil {
250-
nadList = append(nadList, *nad)
251-
}
252-
}
253-
254-
serviceAnnotations, err := nad.EnsureNetworksAnnotation(nadList)
255-
if err != nil {
256-
return ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w",
257-
instance.Spec.NetworkAttachments, err)
221+
serviceAnnotations, ctrlResult, err := r.EnsureNetworkAttachments(
222+
ctx,
223+
Log,
224+
helper,
225+
instance.Spec.NetworkAttachments,
226+
instance.Namespace,
227+
&instance.Status.Conditions,
228+
)
229+
if err != nil || (ctrlResult != ctrl.Result{}) {
230+
return ctrlResult, err
258231
}
259232

260233
// NetworkAttachments
261-
if r.PodExists(ctx, instance, nextWorkflowStep) {
262-
networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(
263-
ctx,
264-
helper,
265-
instance.Spec.NetworkAttachments,
266-
serviceLabels,
267-
1,
268-
)
269-
if err != nil {
270-
return ctrl.Result{}, err
271-
}
272-
273-
instance.Status.NetworkAttachments = networkAttachmentStatus
274-
275-
if networkReady {
276-
instance.Status.Conditions.MarkTrue(
277-
condition.NetworkAttachmentsReadyCondition,
278-
condition.NetworkAttachmentsReadyMessage)
279-
} else {
280-
err := fmt.Errorf("%w: %s", ErrNetworkAttachmentsMismatch, instance.Spec.NetworkAttachments)
281-
instance.Status.Conditions.Set(condition.FalseCondition(
282-
condition.NetworkAttachmentsReadyCondition,
283-
condition.ErrorReason,
284-
condition.SeverityWarning,
285-
condition.NetworkAttachmentsReadyErrorMessage,
286-
err.Error()))
287-
288-
return ctrl.Result{}, err
289-
}
234+
ctrlResult, err = r.VerifyNetworkAttachments(
235+
ctx,
236+
helper,
237+
instance,
238+
instance.Spec.NetworkAttachments,
239+
serviceLabels,
240+
nextWorkflowStep,
241+
&instance.Status.Conditions,
242+
&instance.Status.NetworkAttachments,
243+
)
244+
if err != nil || (ctrlResult != ctrl.Result{}) {
245+
return ctrlResult, err
290246
}
291-
// NetworkAttachments - end
292247

293248
// Create Job
294249
mountCerts := r.CheckSecretExists(ctx, instance, "combined-ca-bundle")

0 commit comments

Comments
 (0)