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

components/ipam-ext: Remove primary-udn-kubevirt-binding NetworkAttachmentDefinition #1949

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

components/ipam-ext: Remove primary-udn-kubevirt-binding NetworkAttachmentDefinition #1949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af0368a
Revert "primary-udn: Match NAD type to binary name (#1851)"
RamLavi Dec 16, 2024
5fbf07a
Revert "Add primary UDN net-attach-def (#1841)"
RamLavi Dec 16, 2024
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
16 changes: 0 additions & 16 deletions data/kubevirt-ipam-controller/004-primary-udn-kubevirt-binding-networkattachdef.yaml
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ require (
github.com/go-git/go-git/v5 v5.11.0
github.com/gobwas/glob v0.2.3
github.com/google/go-github/v32 v32.1.0
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.3.0
github.com/kubevirt/monitoring/pkg/metrics/parser v0.0.0-20231024120544-6a3ba1a680b4
github.com/machadovilaca/operator-observability v0.0.19-0.20240326121036-9f2e5a31675f
github.com/onsi/ginkgo/v2 v2.11.0
Expand Down Expand Up @@ -145,6 +144,7 @@ require (
github.com/josharian/intern v1.0.0 // indirect
github.com/jpillora/backoff v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.3.0 // indirect
github.com/kevinburke/rest v0.0.0-20210506044642-5611499aa33c // indirect
github.com/kevinburke/ssh_config v1.2.0 // indirect
github.com/klauspost/compress v1.16.0 // indirect
Expand Down
2 changes: 1 addition & 1 deletion hack/components/bump-kubevirt-ipam-controller.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ echo 'Adjust kubevirt-ipam-controller to CNAO'

echo 'Copy manifests'
shopt -s extglob
rm -rf data/kubevirt-ipam-controller/!(002-rbac.yaml|004-primary-udn-kubevirt-binding-networkattachdef.yaml)
rm -rf data/kubevirt-ipam-controller/!(002-rbac.yaml)

# CRD
crd_manifest="https://raw.githubusercontent.com/k8snetworkplumbingwg/ipamclaims/${IPAMCLAIMS_CRD_VERSION}/artifacts/k8s.cni.cncf.io_ipamclaims.yaml"
Expand Down
3 changes: 0 additions & 3 deletions pkg/components/components.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,9 +754,6 @@ func GetClusterRole(allowMultus bool) *rbacv1.ClusterRole {
"get",
"list",
"watch",
"create",
"update",
"delete",
},
},
},
Expand Down
45 changes: 0 additions & 45 deletions pkg/network/kubevirt_ipam_controller_test.go
View file
Open in desktop

This file was deleted.

36 changes: 0 additions & 36 deletions test/check/check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
. "github.com/onsi/gomega"

monitoringv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
k8snetworkplumbingwgv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
testenv "github.com/kubevirt/cluster-network-addons-operator/test/env"
conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
securityapi "github.com/openshift/origin/pkg/security/apis/security"
Expand Down Expand Up @@ -347,10 +346,6 @@ func checkForComponent(component *Component) error {
errsAppend(checkForPrometheusRule(component.PrometheusRule))
}

if component.NetworkAttachmentDefinition != "" {
errsAppend(checkForNetworkAttachmentDefinition(component.NetworkAttachmentDefinition))
}

return errsToErr(errs)
}

Expand Down Expand Up @@ -394,10 +389,6 @@ func checkForComponentRemoval(component *Component) error {
errsAppend(checkForPrometheusRuleRemoval(component.PrometheusRule))
}

if component.NetworkAttachmentDefinition != "" {
errsAppend(checkForNetworkAttachmentDefinitionRemoval(component.NetworkAttachmentDefinition))
}

return errsToErr(errs)
}

Expand Down Expand Up @@ -658,21 +649,6 @@ func checkForPrometheusRule(name string) error {
return nil
}

func checkForNetworkAttachmentDefinition(name string) error {
networkAttachmentDefinition := k8snetworkplumbingwgv1.NetworkAttachmentDefinition{}
err := testenv.Client.Get(context.Background(), types.NamespacedName{Name: name, Namespace: corev1.NamespaceDefault}, &networkAttachmentDefinition)
if err != nil {
return err
}

err = checkRelationshipLabels(networkAttachmentDefinition.GetLabels(), "NetworkAttachmentDefinition", name)
if err != nil {
return err
}

return nil
}

func checkRelationshipLabels(labels map[string]string, kind, name string) error {
expectedValues := map[string]string{
names.COMPONENT_LABEL_KEY: names.COMPONENT_LABEL_DEFAULT_VALUE,
Expand Down Expand Up @@ -732,14 +708,6 @@ func checkForPrometheusRuleRemoval(name string) error {
return isNotFound("PrometheusRule", name, err)
}

func checkForNetworkAttachmentDefinitionRemoval(name string) error {
err := testenv.Client.Get(context.Background(), types.NamespacedName{Name: name, Namespace: corev1.NamespaceDefault}, &k8snetworkplumbingwgv1.NetworkAttachmentDefinition{})
if err != nil && isKindNotFound(err) {
return nil
}
return isNotFound("NetworkAttachmentDefinition", name, err)
}

func getMonitoringEndpoint() (*corev1.Endpoints, error) {
By("Finding CNAO prometheus endpoint")
endpoint := &corev1.Endpoints{}
Expand Down Expand Up @@ -844,10 +812,6 @@ func isNotSupportedKind(err error) bool {
return strings.Contains(err.Error(), "no kind is registered for the type")
}

func isKindNotFound(err error) bool {
return strings.Contains(err.Error(), "no matches for kind")
}

func configToYaml(gvk schema.GroupVersionKind) string {
config := GetConfig(gvk)
manifest, err := yaml.Marshal(config)
Expand Down
12 changes: 5 additions & 7 deletions test/check/components.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ type Component struct {
Service string
ServiceMonitor string
PrometheusRule string
NetworkAttachmentDefinition string
}

var (
Expand Down Expand Up @@ -88,12 +87,11 @@ var (
Deployments: []string{"secondary-dns"},
}
KubevirtIpamController = Component{
ComponentName: "KubevirtIpamController",
ClusterRole: "kubevirt-ipam-controller-manager-role",
ClusterRoleBinding: "kubevirt-ipam-controller-manager-rolebinding",
Deployments: []string{"kubevirt-ipam-controller-manager"},
DaemonSets: []string{"passt-binding-cni"},
NetworkAttachmentDefinition: "primary-udn-kubevirt-binding",
ComponentName: "KubevirtIpamController",
ClusterRole: "kubevirt-ipam-controller-manager-role",
ClusterRoleBinding: "kubevirt-ipam-controller-manager-rolebinding",
Deployments: []string{"kubevirt-ipam-controller-manager"},
DaemonSets: []string{"passt-binding-cni"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about removing the ds please ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That should be done on a separate PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

not mandatory imho, but ok if more comfortable for you
as long as the tests use managedTap before that, else they will fail

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So perhaps it is better to do it on the PR that bumps ipam-ext?

Copy link
Collaborator

Choose a reason for hiding this comment

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

nope, orthogonal
passt can be removed after the tests are removed, this way we also each focus on orthogonal parts
as far as possible

flow should be adapt to managedTap,
green to remove passt all over

}
AllComponents = []Component{
KubeMacPoolComponent,
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/workflow/deployment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,9 @@ var _ = Describe("NetworkAddonsConfig", func() {
Entry(
KubevirtIpamController.ComponentName,
cnao.NetworkAddonsConfigSpec{
Multus: &cnao.Multus{},
KubevirtIpamController: &cnao.KubevirtIpamController{},
},
[]Component{MultusComponent, KubevirtIpamController},
[]Component{KubevirtIpamController},
),
)
It("should deploy prometheus if NetworkAddonsConfigSpec is not empty", func() {
Expand Down
3 changes: 0 additions & 3 deletions test/env/env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package env

import (
k8snetworkplumbingwgv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
. "github.com/onsi/gomega"

monitoringv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
Expand Down Expand Up @@ -40,8 +39,6 @@ func Start() {
ExpectWithOffset(1, err).NotTo(HaveOccurred())
err = monitoringv1.AddToScheme(scheme.Scheme)
ExpectWithOffset(1, err).NotTo(HaveOccurred())
err = k8snetworkplumbingwgv1.AddToScheme(scheme.Scheme)
ExpectWithOffset(1, err).NotTo(HaveOccurred())

// +kubebuilder:scaffold:scheme

Expand Down
Loading