Repository: kubernetes/kubernetes
File: test/integration/dra/dra_test.go (Go)

Code before:

func expectedAllocatedClaim(request string, nodeInfo nodeInfo) gtypes.GomegaMatcher {
	return gomega.HaveField("Status.Allocation", gstruct.PointTo(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
		"Devices": gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
			"Results": gomega.HaveExactElements(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
				"Request": gomega.Equal(request),
				"Driver":  gomega.Equal(nodeInfo.driverName),
				"Pool":    gomega.Equal(nodeInfo.pool),
			})),
		}),
		"NodeSelector": gstruct.PointTo(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
			"NodeSelectorTerms": gomega.HaveExactElements(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
				"MatchFields": gomega.HaveExactElements(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
					"Key":      gomega.Equal("metadata.name"),
					"Operator": gomega.Equal(v1.NodeSelectorOpIn),
					"Values":   gomega.HaveExactElements(gomega.Equal(nodeInfo.name)),
				})),
			})),
		})),
	})))
}

func testExtendedResource(tCtx ktesting.TContext, enabled bool, deviceClass *resourceapi.DeviceClass, resourceName string, pod *v1.Pod) {
	tCtx.Parallel()

	namespace := createTestNamespace(tCtx, nil)
	driverName := namespace + driverNameSuffix
	class := deviceClass.DeepCopy()
	class.Spec.Selectors = []resourceapi.DeviceSelector{{
		CEL: &resourceapi.CELDeviceSelector{
			Expression: fmt.Sprintf("device.driver == %q", driverName),
		},
	}}
	_, err := tCtx.Client().ResourceV1().DeviceClasses().Create(tCtx, class, metav1.CreateOptions{FieldValidation: "Strict"})
	tCtx.ExpectNoError(err, "create class")

	slice := st.MakeResourceSlice("worker-0", driverName).Devices(device1)
	createSlice(tCtx, slice.Obj())

	tCtx.Run("scheduler", func(tCtx ktesting.TContext) {
		startScheduler(tCtx)

		pod := pod.DeepCopy()
		pod.Namespace = namespace
		_, err := tCtx.Client().CoreV1().Pods(namespace).Create(tCtx, pod, metav1.CreateOptions{FieldValidation: "Strict"})
		tCtx.ExpectNoError(err, "create pod")
		schedulingAttempted := gomega.HaveField("Status.Conditions", gomega.ContainElement(
			gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
				"Type":    gomega.Equal(v1.PodScheduled),
				"Status":  gomega.Equal(v1.ConditionFalse),
				"Reason":  gomega.Equal("Unschedulable"),

Review comment (suggestion):
This had some value, except that the else branch is missing:
- check  that `c.Spec.ExtendedResourceName` gets dropped
- return, because the rest of the test doesn't do anything useful.

Also, can we replace `c` with `class`? It's just not readable.

I think we should keep this and fix it for the "explicit extended resource" case. For "implicit" it makes no sense to call `testExtendedResource` with `enabled: false`.

IMHO the function is too overloaded now.

Perhaps split out the "explicit + disabled" case into a separate function? It only needs the `DeviceClasses().Create` call and nothing else.




Code after:
	var spec *resourceapi.DeviceClassSpec
	if explicit {
		resourceName = "example.com/" + namespace
		// Set extended resource name in the DeviceClass spec only for explicit resources.
		// It's not required for implicit extended resources.
		spec = &resourceapi.DeviceClassSpec{
			ExtendedResourceName: &resourceName,
		}
	}
	class, driverName := createTestClassWithSpec(tCtx, namespace, spec)
	if explicit {
		if enabled {
			require.NotEmpty(tCtx, class.Spec.ExtendedResourceName, "should store ExtendedResourceName")
		} else {
			require.Empty(tCtx, class.Spec.ExtendedResourceName, "should strip ExtendedResourceName")
		}
	} else {
		// For implicit extended resources, derive the resource name from the class.
		resourceName = resourceapi.ResourceDeviceClassPrefix + class.Name
	}

	slice := st.MakeResourceSlice("worker-0", driverName).Devices(device1)
	createSlice(tCtx, slice.Obj())

	startScheduler(tCtx)

	podWithOneContainer := st.MakePod().Name(podName).Namespace(namespace).Container("test-container").Obj()
	pod := createPodWithExtendedResource(tCtx, namespace, resourceName, "1", podWithOneContainer)

	var schedulingAttempted gtypes.GomegaMatcher
	if enabled {
		// Scheduled using device1 in the slice above.
		schedulingAttempted = gomega.HaveField("Status.Conditions", gomega.ContainElement(
			gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
				"Type":   gomega.Equal(v1.PodScheduled),
				"Status": gomega.Equal(v1.ConditionTrue),
			}),
		))
	} else {
		schedulingAttempted = gomega.HaveField("Status.Conditions", gomega.ContainElement(
			gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
				"Type":    gomega.Equal(v1.PodScheduled),
				"Status":  gomega.Equal(v1.ConditionFalse),
				"Reason":  gomega.Equal("Unschedulable"),
				"Message": gomega.Equal(fmt.Sprintf("0/8 nodes are available: 8 Insufficient %s. no new claims to deallocate, preemption: 0/8 nodes are available: 8 Preemption is not helpful for scheduling.", resourceName)),
			}),
		))
	}
	tCtx.Eventually(func(tCtx ktesting.TContext) (*v1.Pod, error) {
		return tCtx.Client().CoreV1().Pods(namespace).Get(tCtx, pod.Name, metav1.GetOptions{})
	}).WithTimeout(time.Minute).WithPolling(time.Second).Should(schedulingAttempted)