Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow multi NAD in attractor #421

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow multi NAD in attractor #421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions api/v1/attractor_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,6 @@ func (r *Attractor) validateAttractor() error {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec").Child("ipv6-prefix"), "not supported for type"))
}

if len(r.Spec.Interface.NetworkAttachments) != 1 {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("interface").Child("multus-interface"),
r.Spec.Interface.NetworkAttachments, "single Network Attachment item required"))
}
for _, na := range r.Spec.Interface.NetworkAttachments {
if na.Name == "" {
allErrs = append(allErrs, field.Required(field.NewPath("spec").Child("interface").Child("network-attachments").Child("name"),
Expand Down
27 changes: 7 additions & 20 deletions pkg/controllers/attractor/stateless-lb-frontend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,22 +90,13 @@ func (l *LoadBalancer) getNscEnvVars(allEnv []corev1.EnvVar) []corev1.EnvVar {

func (l *LoadBalancer) getFeEnvVars(allEnv []corev1.EnvVar) []corev1.EnvVar {
operatorEnv := map[string]string{
"NFE_CONFIG_MAP_NAME": common.ConfigMapName(l.trench),
"NFE_NSP_SERVICE": common.NSPServiceWithPort(l.trench),
"NFE_TRENCH_NAME": l.trench.ObjectMeta.Name,
"NFE_ATTRACTOR_NAME": l.attractor.ObjectMeta.Name,
"NFE_NAMESPACE": l.attractor.ObjectMeta.Namespace,
"NFE_EXTERNAL_INTERFACE": func() string {
externalInterface := l.attractor.Spec.Interface.Name
// if set use the interface provided by the Network Attachment
if l.attractor.Spec.Interface.Type == meridiov1.NAD &&
len(l.attractor.Spec.Interface.NetworkAttachments) == 1 &&
l.attractor.Spec.Interface.NetworkAttachments[0].InterfaceRequest != "" {
externalInterface = l.attractor.Spec.Interface.NetworkAttachments[0].InterfaceRequest
}
return externalInterface
}(),
"NFE_LOG_LEVEL": common.GetLogLevel(),
"NFE_CONFIG_MAP_NAME": common.ConfigMapName(l.trench),
"NFE_NSP_SERVICE": common.NSPServiceWithPort(l.trench),
"NFE_TRENCH_NAME": l.trench.ObjectMeta.Name,
"NFE_ATTRACTOR_NAME": l.attractor.ObjectMeta.Name,
"NFE_NAMESPACE": l.attractor.ObjectMeta.Namespace,
"NFE_EXTERNAL_INTERFACE": l.attractor.Spec.Interface.Name,
Copy link
Collaborator

@zolug zolug Jun 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember right, when the code appends the annotations, the interface name to be used in the POD is decided based on NAD config in the Attractor. Meaning, if there's no interface name in the NAD, the code will derive the name from the Specs.Interface.Name.
So, based on this change, the "primary" Attractor NAD supplying the external interface must not have an Interface Name set.
Also, in case multiple NADs are set, but none of them have an interface name specified, the code would try to re-use the same Specs.Interface.Name as POD interface names when appending the NADs to the resource template, wouldn't it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the user should give a specific interface name in the non-primary network-attachments in the attractor spec. The primary one should not have any name (or it should match with the .Specs.Interface.Name).

There is an example here: https://gist.github.com/LionelJouin/3cf494db1db592fb01b16ff16b6493c8
The attractor has 2 NADs, and the macvlan is a secondary one, the primary one is the vlan-100 that will get the ext-vlan0 name.

"NFE_LOG_LEVEL": common.GetLogLevel(),
}
return common.CompileEnvironmentVariables(allEnv, operatorEnv)
}
Expand All @@ -114,10 +105,6 @@ func (l *LoadBalancer) getFeEnvVars(allEnv []corev1.EnvVar) []corev1.EnvVar {
// Currently a single Network Attachment is supported, but the code can be easily extended
// to support multiple.
func (l *LoadBalancer) insertNetworkAnnotation(dep *appsv1.Deployment) error {
if len(l.attractor.Spec.Interface.NetworkAttachments) != 1 {
return fmt.Errorf("required one network attachment")
}

if dep.Spec.Template.ObjectMeta.Annotations == nil {
dep.Spec.Template.ObjectMeta.Annotations = make(map[string]string)
}
Expand Down