Skip to content

Commit

Permalink
nfd-master: drop the -nrt-namespace cmdline flag
Browse files Browse the repository at this point in the history
Always create the CR in the namespace we're running in. NFD owns the CR
so it should create it in its own namespace rather than messing around
in other namespaces. Also makes cleanup less error-prone. Deleting the
nfd namespace removes the CRs too, not leaving them abandoned in some
random corner of the cluster.

Update RBAC rules accordingly as nfd-master doesn't require cluster-wide
privileges to NodeResourceTopology objects, anymore.
  • Loading branch information
marquiz committed Oct 4, 2021
1 parent d76af7d commit 54e850f
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 20 deletions.
3 changes: 0 additions & 3 deletions cmd/nfd-master/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,6 @@ func initFlags(flagset *flag.FlagSet) *master.Args {
flagset.BoolVar(&args.VerifyNodeName, "verify-node-name", false,
"Verify worker node name against the worker's TLS certificate. "+
"Only takes effect when TLS authentication has been enabled.")
flagset.StringVar(&args.NRTNamespace, "nrt-namespace", "default",
"Namespace in which Node Resource Topology CR are created"+
"Ensure that the namespace specified is already exists.")

return args
}
2 changes: 2 additions & 0 deletions deployment/base/rbac/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ resources:
- master-serviceaccount.yaml
- master-clusterrole.yaml
- master-clusterrolebinding.yaml
- master-role.yaml
- master-rolebinding.yaml
8 changes: 0 additions & 8 deletions deployment/base/rbac/master-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,3 @@ rules:
- patch
- update
- list
- apiGroups:
- topology.node.k8s.io
resources:
- noderesourcetopologies
verbs:
- create
- get
- update
14 changes: 14 additions & 0 deletions deployment/base/rbac/master-role.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: nfd-master
rules:
- apiGroups:
- topology.node.k8s.io
resources:
- noderesourcetopologies
verbs:
- create
- get
- update

12 changes: 12 additions & 0 deletions deployment/base/rbac/master-rolebinding.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: nfd-master
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: nfd-master
subjects:
- kind: ServiceAccount
name: nfd-master

4 changes: 4 additions & 0 deletions deployment/components/common/env.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: KUBERNETES_NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
19 changes: 10 additions & 9 deletions pkg/nfd-master/nfd-master.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ type Args struct {
Prune bool
VerifyNodeName bool
ResourceLabels utils.StringSetVal
NRTNamespace string
}

type NfdMaster interface {
Expand All @@ -106,6 +105,7 @@ type NfdMaster interface {
type nfdMaster struct {
args Args
nodeName string
namespace string
annotationNs string
server *grpc.Server
stop chan struct{}
Expand All @@ -116,9 +116,10 @@ type nfdMaster struct {
// Create new NfdMaster server instance.
func NewNfdMaster(args *Args) (NfdMaster, error) {
nfd := &nfdMaster{args: *args,
nodeName: os.Getenv("NODE_NAME"),
ready: make(chan bool, 1),
stop: make(chan struct{}, 1),
nodeName: os.Getenv("NODE_NAME"),
namespace: os.Getenv("KUBERNETES_NAMESPACE"),
ready: make(chan bool, 1),
stop: make(chan struct{}, 1),
}

if args.Instance == "" {
Expand Down Expand Up @@ -447,7 +448,7 @@ func (m *nfdMaster) UpdateNodeTopology(c context.Context, r *topologypb.NodeTopo
klog.Infof("received CR updation request for node %q", r.NodeName)
}
if !m.args.NoPublish {
err := m.updateCR(r.NodeName, r.TopologyPolicies, r.Zones, m.args.NRTNamespace)
err := m.updateCR(r.NodeName, r.TopologyPolicies, r.Zones)
if err != nil {
klog.Errorf("failed to advertise NodeResourceTopology: %w", err)
return &topologypb.NodeTopologyResponse{}, err
Expand Down Expand Up @@ -642,15 +643,15 @@ func modifyCR(topoUpdaterZones []*v1alpha1.Zone) []v1alpha1.Zone {
return zones
}

func (m *nfdMaster) updateCR(hostname string, tmpolicy []string, topoUpdaterZones []*v1alpha1.Zone, namespace string) error {
func (m *nfdMaster) updateCR(hostname string, tmpolicy []string, topoUpdaterZones []*v1alpha1.Zone) error {
cli, err := m.apihelper.GetTopologyClient()
if err != nil {
return err
}

zones := modifyCR(topoUpdaterZones)

nrt, err := cli.TopologyV1alpha1().NodeResourceTopologies(namespace).Get(context.TODO(), hostname, metav1.GetOptions{})
nrt, err := cli.TopologyV1alpha1().NodeResourceTopologies(m.namespace).Get(context.TODO(), hostname, metav1.GetOptions{})
if errors.IsNotFound(err) {
nrtNew := v1alpha1.NodeResourceTopology{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -660,7 +661,7 @@ func (m *nfdMaster) updateCR(hostname string, tmpolicy []string, topoUpdaterZone
TopologyPolicies: tmpolicy,
}

_, err := cli.TopologyV1alpha1().NodeResourceTopologies(namespace).Create(context.TODO(), &nrtNew, metav1.CreateOptions{})
_, err := cli.TopologyV1alpha1().NodeResourceTopologies(m.namespace).Create(context.TODO(), &nrtNew, metav1.CreateOptions{})
if err != nil {
return fmt.Errorf("failed to create v1alpha1.NodeResourceTopology!:%w", err)
}
Expand All @@ -672,7 +673,7 @@ func (m *nfdMaster) updateCR(hostname string, tmpolicy []string, topoUpdaterZone
nrtMutated := nrt.DeepCopy()
nrtMutated.Zones = zones

nrtUpdated, err := cli.TopologyV1alpha1().NodeResourceTopologies(namespace).Update(context.TODO(), nrtMutated, metav1.UpdateOptions{})
nrtUpdated, err := cli.TopologyV1alpha1().NodeResourceTopologies(m.namespace).Update(context.TODO(), nrtMutated, metav1.UpdateOptions{})
if err != nil {
return fmt.Errorf("failed to update v1alpha1.NodeResourceTopology!:%w", err)
}
Expand Down

0 comments on commit 54e850f

Please sign in to comment.