Skip to content

Commit

Permalink
Exclude DaemonSets (PEMs) from modification by a Vizier's nodeSelector (
Browse files Browse the repository at this point in the history
#1887)

Summary: Exclude DaemonSets (PEMs) from modification by a Vizier's
nodeSelector

This accomplishes part of #1861. The remaining work is to include new
additional node selector configuration values that only apply to PEMs --
similar to how `pemMemoryLimit` and `pemMemoryRequest` work. From my
investigation so far, it appears that adding the PEM specific selectors
will require a different implementation (via the vizier yaml templating
on the Pixie cloud side) and so I thought staging this in two changes
made sense.

Relevant Issues: #1861

Type of change: /kind bug

Test Plan: Skaffolded a vizier operator and verified that setting a
`kubernetes.io/hostname` node selector no longer prevents PEMs from
being scheduled on all nodes

Changelog Message: Fixed an issue that caused the `Vizier` CRD to apply
node selectors to pods that should be scheduled on all nodes
(DaemonSets)

---------

Signed-off-by: Dom Delnano <[email protected]>
  • Loading branch information
ddelnano authored May 9, 2024
1 parent cd22e1a commit 79886a4
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 10 deletions.
7 changes: 7 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ output:
issues:
max-issues-per-linter: 0
max-same-issues: 0
# TODO(ddelnano): Remove once typecheck is upgraded in next golangci-lint upgrade
# This error originates from the stdlib due to generics usage
exclude-rules:
- path: .*slices\/sort.go
linters:
- typecheck
text: "^(undefined: (min|max))"

linters:
enable:
Expand Down
31 changes: 21 additions & 10 deletions src/operator/controllers/vizier_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"encoding/json"
"errors"
"fmt"
"slices"
"strings"
"time"

Expand Down Expand Up @@ -73,6 +74,11 @@ const (
// a storage class is default.
var defaultClassAnnotationKeys = []string{"storageclass.kubernetes.io/is-default-class", "storageclass.beta.kubernetes.io/is-default-class"}

// The k8s API kinds that should be excluded from a Vizier's nodeSelector setting.
// Resources such as DaemonSets should run on all nodes of the cluster, so applying
// the nodeSelector uniformly across all pods leads to unexpected behavior.
var nodeSelectorExcludedKinds = []string{"DaemonSet"}

// VizierReconciler reconciles a Vizier object
type VizierReconciler struct {
client.Client
Expand Down Expand Up @@ -986,6 +992,8 @@ func convertTolerations(tolerations []v1.Toleration) []*vizierconfigpb.Toleratio
}

func updatePodSpec(nodeSelector map[string]string, tolerations []v1.Toleration, securityCtx *v1alpha1.PodSecurityContext, res map[string]interface{}) {
kind, kOk := res["kind"].(string)

podSpec := make(map[string]interface{})
md, ok, err := unstructured.NestedFieldNoCopy(res, "spec", "template", "spec")
if ok && err == nil {
Expand All @@ -994,18 +1002,21 @@ func updatePodSpec(nodeSelector map[string]string, tolerations []v1.Toleration,
}
}

castedNodeSelector := make(map[string]interface{})
ns, ok := podSpec["nodeSelector"].(map[string]interface{})
if ok {
castedNodeSelector = ns
}
for k, v := range nodeSelector {
if _, ok := castedNodeSelector[k]; ok {
continue
if kOk && !slices.Contains(nodeSelectorExcludedKinds, kind) {
castedNodeSelector := make(map[string]interface{})
ns, ok := podSpec["nodeSelector"].(map[string]interface{})
if ok {
castedNodeSelector = ns
}
castedNodeSelector[k] = v
for k, v := range nodeSelector {
if _, ok := castedNodeSelector[k]; ok {
continue
}
castedNodeSelector[k] = v
}
podSpec["nodeSelector"] = castedNodeSelector
}
podSpec["nodeSelector"] = castedNodeSelector

podSpec["tolerations"] = tolerations

// Add securityContext only if enabled.
Expand Down

0 comments on commit 79886a4

Please sign in to comment.