[slice] e2e tests still passing after removing health label

[gabesaba]

We annotate each replicated job in JobSet here:

xpk/slice/internal/webhooks/jobset_webhook.go

Line 66 in 1d2ac6b

annotateReplicatedJobWithSliceHealth(rj)

However, after deletion/modification of the label used in e2e test, the tests still pass. Remove this line:

xpk/slice/hack/kind-cluster.yaml

Line 30 in 1d2ac6b

cloud.google.com/gke-tpu-slice-4x4x4-health: true

And observe e2e tests still pass. Kueue should not return topology assignments on such nodes for these replicated jobs, which require this NodeLabel

Additional Info
Modifying the loop which creates each replicated job to add a node selector:

jt.Spec.Template.Spec.NodeSelector = map[string]string{"hello": "world"}

Results in Kueue's e2e tests failing. So, it seems that Kueue/TAS is acting upon these node selectors in some way. This makes me think that this is more likely a Slice Controller issue.

[mbobrovskyi]

I also tested this case in the Kueue project. The problem isn't with the slice project – it's in the Kueue project. For some reason, filtering by NodeSelector works with three-level topology but not with two-level topology. I'm researching why this happens.

[mbobrovskyi]

I think I identified the issue. Thanks, @mimowo, for the help. We need to add a third low-level label, "kubernetes.io/hostname," to enable filtering by NodeSelector. Otherwise, the filtering doesn’t work. I tested it locally, and it works with the third level. @gabesaba should we update the tests?

[gabesaba]

Do we consider this WAI from the Kueue side @mimowo?

[gabesaba]

@gabesaba should we update the tests?

Yes, please!

[mimowo]

Yeah, it is undefined behavior currently what happens when a user does not set hostname as the lowest level.

The implementation now ignores the node labels and node taints, because in the Topology domain like rack may have a mix of nodes with different taints and labels, so we miss the granularity level.

Ideally I think we should always create the virtual level for hostname even if not specified by user explicitly in Topology, but it has never prioritized. Basically for now we recommend always setting hostname as the lowest level.

cc @mwysokin

[mbobrovskyi]

After adding the third level to the topology, I got a status of TopologyAssignment with only kubernetes.io/hostname level:

  status:
    admission:
      clusterQueue: cq
      podSetAssignments:
      - count: 16
        flavors:
          cloud.google.com/tpu: rf
        name: rj1
        resourceUsage:
          cloud.google.com/tpu: "64"
        topologyAssignment:
          domains:
          - count: 16
            values:
            - kind-worker
          levels:
          - kubernetes.io/hostname

If I correct understood this is correct behavior, but we have validation:

xpk/slice/internal/topology/validation.go

Lines 44 to 46 in 9c41199

	func IsAssignmentValid(psa kueue.PodSetAssignment) bool {
	return psa.TopologyAssignment != nil && SubblockLevelIndex(psa.TopologyAssignment) != -1
	}

xpk/slice/internal/topology/topology.go

Lines 23 to 32 in 9c41199

	// SubblockLevelIndex returns the index of the TPUSubBlock topology
	// assignment, or -1 if it doesn't exist.
	func SubblockLevelIndex(topologyAssignment *kueue.TopologyAssignment) int {
	for i, level := range topologyAssignment.Levels {
	if level == core.TPUSubBlockLabel {
	return i
	}
	}
	return -1
	}

This validation doesn't allow to create slice due to an error:

2025-09-11T11:22:54Z	LEVEL(-3)	Skipping workload as it has invalid topology assignments	{"controller": "workload_controller", "controllerGroup": "kueue.x-k8s.io", "controllerKind": "Workload", "Workload": {"name":"jobset-jobset-4bf7e","namespace":"e2e-jobset-n29tm"}, "namespace": "e2e-jobset-n29tm", "name": "jobset-jobset-4bf7e", "reconcileID": "3f86571f-2b86-493a-8b60-99bea8afbfdd"}

Should we delete this validation?
@gabesaba WDYT?

[gabesaba]

That doesn't sound correct, as we need the Subblocks to create the slice, after the topology assignment. Are you not seeing different failures later, when you remove that validation?

[mbobrovskyi]

That doesn't sound correct, as we need the Subblocks to create the slice, after the topology assignment. Are you not seeing different failures later, when you remove that validation?

Oh, I stopped at this step and didn’t check the next ones.

[mbobrovskyi]

Than we can't use third level, right?

[mbobrovskyi]

@gabesaba @mwysokin friendly ping 🙏

[gabesaba]

Let's do this:

1. Use 3-level topology
2. Parse node assignments back into sub-blocks for creation of slice CRD
   a) requires listing nodes, if we don't already do that

Does this seem feasible?

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.