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.

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

Add owner reference to NRT object #1602

Merged
merged 1 commit into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ rules:
verbs:
- get
- list
- apiGroups:
- ""
resources:
- namespaces
verbs:
- get
- apiGroups:
- ""
resources:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ rules:
verbs:
- get
- list
- apiGroups:
- ""
resources:
- namespaces
verbs:
- get
- apiGroups:
- ""
resources:
Expand Down
28 changes: 27 additions & 1 deletion pkg/nfd-topology-updater/nfd-topology-updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
k8sclient "k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"
Expand Down Expand Up @@ -80,6 +81,9 @@
eventSource <-chan kubeletnotifier.Info
configFilePath string
config *NFDConfig
kubernetesNamespace string
ownerRefs []metav1.OwnerReference
k8sClient k8sclient.Interface
kubeletConfigFunc func() (*kubeletconfigv1beta1.KubeletConfiguration, error)
}

Expand All @@ -105,6 +109,8 @@
nodeName: utils.NodeName(),
eventSource: eventSource,
config: &NFDConfig{},
kubernetesNamespace: utils.GetKubernetesNamespace(),
ownerRefs: []metav1.OwnerReference{},

Check warning on line 113 in pkg/nfd-topology-updater/nfd-topology-updater.go

View check run for this annotation

Codecov / codecov/patch

pkg/nfd-topology-updater/nfd-topology-updater.go#L112-L113

Added lines #L112 - L113 were not covered by tests
kubeletConfigFunc: kubeletConfigFunc,
}
if args.ConfigFile != "" {
Expand Down Expand Up @@ -146,6 +152,7 @@
if err != nil {
return err
}
w.k8sClient = k8sClient

Check warning on line 155 in pkg/nfd-topology-updater/nfd-topology-updater.go

View check run for this annotation

Codecov / codecov/patch

pkg/nfd-topology-updater/nfd-topology-updater.go#L155

Added line #L155 was not covered by tests

if err := w.configure(); err != nil {
return fmt.Errorf("faild to configure Node Feature Discovery Topology Updater: %w", err)
Expand Down Expand Up @@ -222,11 +229,29 @@
}

func (w *nfdTopologyUpdater) updateNodeResourceTopology(zoneInfo v1alpha2.ZoneList, scanResponse resourcemonitor.ScanResponse, readKubeletConfig bool) error {

if len(w.ownerRefs) == 0 {
ns, err := w.k8sClient.CoreV1().Namespaces().Get(context.TODO(), w.kubernetesNamespace, metav1.GetOptions{})
if err != nil {
klog.ErrorS(err, "Cannot get NodeResourceTopology owner reference")
} else {
w.ownerRefs = []metav1.OwnerReference{
{
APIVersion: ns.APIVersion,
Kind: ns.Kind,
Name: ns.Name,
UID: types.UID(ns.UID),
},
}
}

Check warning on line 246 in pkg/nfd-topology-updater/nfd-topology-updater.go

View check run for this annotation

Codecov / codecov/patch

pkg/nfd-topology-updater/nfd-topology-updater.go#L232-L246

Added lines #L232 - L246 were not covered by tests
}

nrt, err := w.topoClient.TopologyV1alpha2().NodeResourceTopologies().Get(context.TODO(), w.nodeName, metav1.GetOptions{})
if errors.IsNotFound(err) {
nrtNew := v1alpha2.NodeResourceTopology{
ObjectMeta: metav1.ObjectMeta{
Name: w.nodeName,
Name: w.nodeName,
OwnerReferences: w.ownerRefs,

Check warning on line 254 in pkg/nfd-topology-updater/nfd-topology-updater.go

View check run for this annotation

Codecov / codecov/patch

pkg/nfd-topology-updater/nfd-topology-updater.go#L253-L254

Added lines #L253 - L254 were not covered by tests
},
Zones: zoneInfo,
Attributes: v1alpha2.AttributeList{},
Expand All @@ -248,6 +273,7 @@

nrtMutated := nrt.DeepCopy()
nrtMutated.Zones = zoneInfo
nrtMutated.OwnerReferences = w.ownerRefs

Check warning on line 276 in pkg/nfd-topology-updater/nfd-topology-updater.go

View check run for this annotation

Codecov / codecov/patch

pkg/nfd-topology-updater/nfd-topology-updater.go#L276

Added line #L276 was not covered by tests

attributes := scanResponse.Attributes

Expand Down
38 changes: 38 additions & 0 deletions test/e2e/topology_updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,44 @@ var _ = NFDDescribe(Label("nfd-topology-updater"), func() {
return true
}, time.Minute, 5*time.Second).Should(BeTrue(), "didn't get updated node topology info")
})

ozhuraki marked this conversation as resolved.
Show resolved Hide resolved
It("should check that that topology object is garbage colleted", func(ctx context.Context) {

By("Check if the topology object has owner reference")
ns, err := f.ClientSet.CoreV1().Namespaces().Get(ctx, f.Namespace.Name, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())

t, err := topologyClient.TopologyV1alpha2().NodeResourceTopologies().Get(ctx, topologyUpdaterNode.Name, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())

owned := false
for _, r := range t.OwnerReferences {
if r.UID == ns.UID {
owned = true
break
}
}
Expect(owned).Should(BeTrue())

By("Deleting the nfd-topology namespace")
err = f.ClientSet.CoreV1().Namespaces().Delete(ctx, f.Namespace.Name, metav1.DeleteOptions{})
Expect(err).NotTo(HaveOccurred())

By("checking that topology was garbage collected")
Eventually(func() bool {
t, err := topologyClient.TopologyV1alpha2().NodeResourceTopologies().Get(ctx, topologyUpdaterNode.Name, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
framework.Logf("missing node topology resource for %q", topologyUpdaterNode.Name)
return false
}
framework.Logf("failed to get the node topology resource: %v", err)
return true
}
framework.Logf("topology resource: %v", t)
return true
}).WithPolling(15 * time.Second).WithTimeout(60 * time.Second).Should(BeFalse())
})
})

When("sleep interval disabled", func() {
Expand Down
27 changes: 21 additions & 6 deletions test/e2e/utils/rbac.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clientset "k8s.io/client-go/kubernetes"
)
Expand Down Expand Up @@ -105,10 +106,6 @@ func DeconfigureRBAC(ctx context.Context, cs clientset.Interface, ns string) err
if err != nil {
return err
}
err = cs.RbacV1().RoleBindings(ns).Delete(ctx, "nfd-worker-e2e", metav1.DeleteOptions{})
if err != nil {
return err
}
err = cs.RbacV1().ClusterRoleBindings().Delete(ctx, "nfd-gc-e2e", metav1.DeleteOptions{})
if err != nil {
return err
Expand All @@ -121,11 +118,24 @@ func DeconfigureRBAC(ctx context.Context, cs clientset.Interface, ns string) err
if err != nil {
return err
}
err = cs.RbacV1().Roles(ns).Delete(ctx, "nfd-worker-e2e", metav1.DeleteOptions{})
err = cs.RbacV1().ClusterRoles().Delete(ctx, "nfd-gc-e2e", metav1.DeleteOptions{})
if err != nil {
return err
}
err = cs.RbacV1().ClusterRoles().Delete(ctx, "nfd-gc-e2e", metav1.DeleteOptions{})

// Skip the deletion of namespaced objects in case the namespace is gone
_, err = cs.CoreV1().Namespaces().Get(ctx, ns, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
return nil
}
return err
}
err = cs.RbacV1().RoleBindings(ns).Delete(ctx, "nfd-worker-e2e", metav1.DeleteOptions{})
if err != nil {
return err
}
err = cs.RbacV1().Roles(ns).Delete(ctx, "nfd-worker-e2e", metav1.DeleteOptions{})
if err != nil {
return err
}
Expand Down Expand Up @@ -251,6 +261,11 @@ func createClusterRoleTopologyUpdater(ctx context.Context, cs clientset.Interfac
Resources: []string{"pods"},
Verbs: []string{"get", "list", "watch"},
},
{
APIGroups: []string{""},
Resources: []string{"namespaces"},
Verbs: []string{"get"},
},
{
APIGroups: []string{"topology.node.k8s.io"},
Resources: []string{"noderesourcetopologies"},
Expand Down