Skip to content

Commit

Permalink
Merge pull request openstack-k8s-operators#262 from elvgarrui/revert_…
Browse files Browse the repository at this point in the history 
…networkattachments

Revert "Add support for adding multiple networkattachments to the ovn…
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Apr 9, 2024
2 parents 765fc25 + d5ce36b commit 4d443b8
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 283 deletions.
9 changes: 2 additions & 7 deletions api/bases/ovn.openstack.org_ovncontrollers.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,10 @@ spec:
type: string
type: object
networkAttachment:
description: 'NetworkAttachment is a NetworkAttachment resource name
description: NetworkAttachment is a NetworkAttachment resource name
to expose the service to the given network. If specified the IP
address of this network is used as the OvnEncapIP. Deprecated: superseded
by NetworkAttachments'
address of this network is used as the OvnEncapIP.
type: string
networkAttachments:
items:
type: string
type: array
nicMappings:
additionalProperties:
type: string
Expand Down
7 changes: 0 additions & 7 deletions api/v1beta1/ovncontroller_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,8 @@ type OVNControllerSpecCore struct {
// +kubebuilder:validation:Optional
// NetworkAttachment is a NetworkAttachment resource name to expose the service to the given network.
// If specified the IP address of this network is used as the OvnEncapIP.
// Deprecated: superseded by NetworkAttachments
NetworkAttachment string `json:"networkAttachment"`

// +kubebuilder:validation:Optional
// NetworkAttachments are NetworkAttachment resources used to expose the service to the specified networks.
// If present, the IP of the attachment named "tenant", will be used as the OvnEncapIP.

NetworkAttachments []string `json:"networkAttachments,omitempty"`

// +kubebuilder:validation:Optional
// +operator-sdk:csv:customresourcedefinitions:type=spec
// TLS - Parameters related to TLS
Expand Down
5 changes: 0 additions & 5 deletions api/v1beta1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 2 additions & 7 deletions config/crd/bases/ovn.openstack.org_ovncontrollers.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,10 @@ spec:
type: string
type: object
networkAttachment:
description: 'NetworkAttachment is a NetworkAttachment resource name
description: NetworkAttachment is a NetworkAttachment resource name
to expose the service to the given network. If specified the IP
address of this network is used as the OvnEncapIP. Deprecated: superseded
by NetworkAttachments'
address of this network is used as the OvnEncapIP.
type: string
networkAttachments:
items:
type: string
type: array
nicMappings:
additionalProperties:
type: string
Expand Down
31 changes: 5 additions & 26 deletions controllers/ovncontroller_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,23 +59,6 @@ import (
rbacv1 "k8s.io/api/rbac/v1"
)

const (
// TunnelNetworkAttachment is the name of the network attachment
// that will be used for overlay tenant networking. If present in
// the list of attachments, OvnEncapIP is set to this interface's
// IP.
TunnelNetworkAttachmentName = "tenant"
)

func contains(s []string, str string) bool {
for _, v := range s {
if v == str {
return true
}
}
return false
}

// getlog returns a logger object with a prefix of "conroller.name" and aditional controller context fields
func (r *OVNControllerReconciler) GetLogger(ctx context.Context) logr.Logger {
return log.FromContext(ctx).WithName("Controllers").WithName("OVNController")
Expand Down Expand Up @@ -466,14 +449,12 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance
return ctrl.Result{}, err
}

// network to attach to
networkAttachmentsNoPhysNet := []string{}
if instance.Spec.NetworkAttachments != nil && len(instance.Spec.NetworkAttachments) > 0 {
networkAttachmentsNoPhysNet = append(networkAttachmentsNoPhysNet, instance.Spec.NetworkAttachments...)
}
if instance.Spec.NetworkAttachment != "" && !contains(networkAttachmentsNoPhysNet, instance.Spec.NetworkAttachment) {
if instance.Spec.NetworkAttachment != "" {
networkAttachments = append(networkAttachments, instance.Spec.NetworkAttachment)
networkAttachmentsNoPhysNet = append(networkAttachmentsNoPhysNet, instance.Spec.NetworkAttachment)
}
networkAttachments = append(networkAttachments, networkAttachmentsNoPhysNet...)
sort.Strings(networkAttachments)

for _, netAtt := range networkAttachments {
Expand Down Expand Up @@ -591,7 +572,7 @@ func (r *OVNControllerReconciler) reconcileNormal(ctx context.Context, instance
if networkReady {
instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage)
} else {
err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %v", networkAttachmentsNoPhysNet)
err := fmt.Errorf("not all pods have interfaces with ips as configured in NetworkAttachments: %s", instance.Spec.NetworkAttachment)
instance.Status.Conditions.Set(condition.FalseCondition(
condition.NetworkAttachmentsReadyCondition,
condition.ErrorReason,
Expand Down Expand Up @@ -710,9 +691,7 @@ func (r *OVNControllerReconciler) generateServiceConfigMaps(
cmLabels := labels.GetLabels(instance, labels.GetGroupLabel(ovnv1.ServiceNameOvnController), map[string]string{})

templateParameters := make(map[string]interface{})
if contains(instance.Spec.NetworkAttachments, TunnelNetworkAttachmentName) {
templateParameters["OvnEncapNIC"] = nad.GetNetworkIFName(TunnelNetworkAttachmentName)
} else if instance.Spec.NetworkAttachment != "" {
if instance.Spec.NetworkAttachment != "" {
templateParameters["OvnEncapNIC"] = nad.GetNetworkIFName(instance.Spec.NetworkAttachment)
} else {
templateParameters["OvnEncapNIC"] = "eth0"
Expand Down
233 changes: 2 additions & 231 deletions tests/functional/ovncontroller_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ var _ = Describe("OVNController controller", func() {
corev1.ConditionFalse,
condition.ErrorReason,
"NetworkAttachments error occurred "+
"not all pods have interfaces with ips as configured in NetworkAttachments: [internalapi]",
"not all pods have interfaces with ips as configured in NetworkAttachments: internalapi",
)
})
It("reports that an IP is missing", func() {
Expand Down Expand Up @@ -408,7 +408,7 @@ var _ = Describe("OVNController controller", func() {
corev1.ConditionFalse,
condition.ErrorReason,
"NetworkAttachments error occurred "+
"not all pods have interfaces with ips as configured in NetworkAttachments: [internalapi]",
"not all pods have interfaces with ips as configured in NetworkAttachments: internalapi",
)
})
It("reports NetworkAttachmentsReady if the Pods got the proper annotations", func() {
Expand Down Expand Up @@ -731,235 +731,6 @@ var _ = Describe("OVNController controller", func() {
})
})

When("OVNController is created with networkAttachments and nic configs", func() {
BeforeEach(func() {
dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
DeferCleanup(DeleteOVNDBClusters, dbs)
spec := GetDefaultOVNControllerSpec()
spec.NetworkAttachments = []string{"internalapi"}
spec.NicMappings = map[string]string{
"physnet1": "enp2s0.100",
}
instance := CreateOVNController(namespace, spec)
DeferCleanup(th.DeleteInstance, instance)
})

It("reports that daemonset has annotations for both Networkattachment and nic-configs", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)

daemonSetName := types.NamespacedName{
Namespace: namespace,
Name: "ovn-controller",
}
daemonSetNameOVS := types.NamespacedName{
Namespace: namespace,
Name: "ovn-controller-ovs",
}

ds := GetDaemonSet(daemonSetName)
Expect(ds.Spec.Template.ObjectMeta.Annotations).To(BeNil())

ds = GetDaemonSet(daemonSetNameOVS)
expectedAnnotation, err := json.Marshal(
[]networkv1.NetworkSelectionElement{
{
Name: "internalapi",
Namespace: namespace,
InterfaceRequest: "internalapi",
},
{
Name: "physnet1",
Namespace: namespace,
InterfaceRequest: "physnet1",
},
})
Expect(err).ShouldNot(HaveOccurred())
Expect(ds.Spec.Template.ObjectMeta.Annotations).To(
HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)),
)
})
})

When("OVNController is created with old networkAttachment and new networkAttachments and nic configs", func() {
BeforeEach(func() {
dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
DeferCleanup(DeleteOVNDBClusters, dbs)
spec := GetDefaultOVNControllerSpec()
spec.NetworkAttachment = "tenant"
spec.NetworkAttachments = []string{"internalapi"}
spec.NicMappings = map[string]string{
"physnet1": "enp2s0.100",
}
instance := CreateOVNController(namespace, spec)
DeferCleanup(th.DeleteInstance, instance)
})

It("reports that daemonset has annotations for both Networkattachment and nic-configs", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)
tenantNADName := types.NamespacedName{Namespace: namespace, Name: "tenant"}
nad = th.CreateNetworkAttachmentDefinition(tenantNADName)
DeferCleanup(th.DeleteInstance, nad)

daemonSetName := types.NamespacedName{
Namespace: namespace,
Name: "ovn-controller",
}
daemonSetNameOVS := types.NamespacedName{
Namespace: namespace,
Name: "ovn-controller-ovs",
}
ds := GetDaemonSet(daemonSetName)

Expect(ds.Spec.Template.ObjectMeta.Annotations).To(BeNil())
ds = GetDaemonSet(daemonSetNameOVS)
expectedAnnotation, err := json.Marshal(
[]networkv1.NetworkSelectionElement{
{
Name: "internalapi",
Namespace: namespace,
InterfaceRequest: "internalapi",
},
{
Name: "physnet1",
Namespace: namespace,
InterfaceRequest: "physnet1",
},
{
Name: "tenant",
Namespace: namespace,
InterfaceRequest: "tenant",
},
})
Expect(err).ShouldNot(HaveOccurred())
Expect(ds.Spec.Template.ObjectMeta.Annotations).To(
HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)),
)
})
})

When("OVNController is created with networkAttachments and nic configs", func() {
BeforeEach(func() {
dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
DeferCleanup(DeleteOVNDBClusters, dbs)
spec := GetDefaultOVNControllerSpec()
spec.NetworkAttachments = []string{"internalapi", "tenant"}
spec.NicMappings = map[string]string{
"physnet1": "enp2s0.100",
}
instance := CreateOVNController(namespace, spec)
DeferCleanup(th.DeleteInstance, instance)
})

It("reports that daemonset has annotations for both Networkattachment and nic-configs", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)
tenantNADName := types.NamespacedName{Namespace: namespace, Name: "tenant"}
nad = th.CreateNetworkAttachmentDefinition(tenantNADName)
DeferCleanup(th.DeleteInstance, nad)

daemonSetName := types.NamespacedName{
Namespace: namespace,
Name: "ovn-controller",
}
daemonSetNameOVS := types.NamespacedName{
Namespace: namespace,
Name: "ovn-controller-ovs",
}
ds := GetDaemonSet(daemonSetName)

expectedAnnotation, err := json.Marshal(
[]networkv1.NetworkSelectionElement{
{
Name: "internalapi",
Namespace: namespace,
InterfaceRequest: "internalapi",
},
{
Name: "physnet1",
Namespace: namespace,
InterfaceRequest: "physnet1",
},
{
Name: "tenant",
Namespace: namespace,
InterfaceRequest: "tenant",
},
})
Expect(err).ShouldNot(HaveOccurred())
Expect(ds.Spec.Template.ObjectMeta.Annotations).To(BeNil())

ds = GetDaemonSet(daemonSetNameOVS)
Expect(ds.Spec.Template.ObjectMeta.Annotations).To(
HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)),
)
})
})

When("OVNController is created with old networkAttachment and new networkAttachments (shared value) and nic configs", func() {
BeforeEach(func() {
dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
DeferCleanup(DeleteOVNDBClusters, dbs)
spec := GetDefaultOVNControllerSpec()
spec.NetworkAttachment = "tenant"
spec.NetworkAttachments = []string{"internalapi", "tenant"}
spec.NicMappings = map[string]string{
"physnet1": "enp2s0.100",
}
instance := CreateOVNController(namespace, spec)
DeferCleanup(th.DeleteInstance, instance)
})

It("reports that daemonset has annotations for both Networkattachment and nic-configs", func() {
internalAPINADName := types.NamespacedName{Namespace: namespace, Name: "internalapi"}
nad := th.CreateNetworkAttachmentDefinition(internalAPINADName)
DeferCleanup(th.DeleteInstance, nad)
tenantNADName := types.NamespacedName{Namespace: namespace, Name: "tenant"}
nad = th.CreateNetworkAttachmentDefinition(tenantNADName)
DeferCleanup(th.DeleteInstance, nad)

daemonSetName := types.NamespacedName{
Namespace: namespace,
Name: "ovn-controller",
}
daemonSetNameOVS := types.NamespacedName{
Namespace: namespace,
Name: "ovn-controller-ovs",
}
ds := GetDaemonSet(daemonSetName)

Expect(ds.Spec.Template.ObjectMeta.Annotations).To(BeNil())
expectedAnnotation, err := json.Marshal(
[]networkv1.NetworkSelectionElement{
{
Name: "internalapi",
Namespace: namespace,
InterfaceRequest: "internalapi",
},
{
Name: "physnet1",
Namespace: namespace,
InterfaceRequest: "physnet1",
},
{
Name: "tenant",
Namespace: namespace,
InterfaceRequest: "tenant",
},
})
Expect(err).ShouldNot(HaveOccurred())
ds = GetDaemonSet(daemonSetNameOVS)
Expect(ds.Spec.Template.ObjectMeta.Annotations).To(
HaveKeyWithValue("k8s.v1.cni.cncf.io/networks", string(expectedAnnotation)),
)

})
})

When("OVNController is created with empty spec", func() {
var ovnControllerName types.NamespacedName

Expand Down

0 comments on commit 4d443b8

Please sign in to comment.