PR comments

aws · Jun 14, 2022 · 34cdf72 · 34cdf72
1 parent c2388d4
commit 34cdf72
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 22 deletions.
diff --git a/pkg/controllers/provisioning/scheduling/inflightnode.go b/pkg/controllers/provisioning/scheduling/inflightnode.go
@@ -95,10 +95,7 @@ func (n *InFlightNode) Add(pod *v1.Pod) error {
 	}
 
 	// determine the number of volumes that will be mounted if the pod schedules
-	mountedVolumeCount, err := n.volumeUsage.Validate(pod)
-	if err != nil {
-		return err
-	}
+	mountedVolumeCount := n.volumeUsage.Validate(pod)
 	if mountedVolumeCount > n.maxVolumes {
 		return fmt.Errorf("%d would exceed max volume count of %d", mountedVolumeCount, n.maxVolumes)
 	}

diff --git a/pkg/controllers/provisioning/scheduling/node.go b/pkg/controllers/provisioning/scheduling/node.go
@@ -95,10 +95,7 @@ func (n *Node) Add(pod *v1.Pod) error {
 	// Check instance type combinations
 	requests := resources.Merge(n.requests, resources.RequestsForPods(pod))
 	// determine the number of volumes that will be mounted if the pod schedules
-	mountedVolumeCount, err := n.volumeUsage.Validate(pod)
-	if err != nil {
-		return err
-	}
+	mountedVolumeCount := n.volumeUsage.Validate(pod)
 	instanceTypes := filterInstanceTypes(n.InstanceTypeOptions, nodeRequirements, requests, mountedVolumeCount)
 	if len(instanceTypes) == 0 {
 		return fmt.Errorf("no instance type satisfied resources %s and requirements %s", resources.String(resources.RequestsForPods(pod)), nodeRequirements)

diff --git a/pkg/scheduling/volumelimits.go b/pkg/scheduling/volumelimits.go
@@ -21,7 +21,6 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"knative.dev/pkg/logging"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -43,33 +42,29 @@ func NewVolumeLimits(ctx context.Context) *VolumeLimits {
 }
 
 func (v *VolumeLimits) Add(pod *v1.Pod) {
-	podVolumes, err := v.validate(pod)
-	if err != nil {
-		logging.FromContext(v.ctx).Errorf("inconsistent state registering volume li mit, %s", err)
-	}
+	podVolumes := v.validate(pod)
 	v.podVolumes[client.ObjectKeyFromObject(pod)] = podVolumes
 	v.volumes = v.volumes.Union(podVolumes)
 }
 
-func (v *VolumeLimits) Validate(pod *v1.Pod) (int, error) {
-	podVolumes, err := v.validate(pod)
+func (v *VolumeLimits) Validate(pod *v1.Pod) int {
+	podVolumes := v.validate(pod)
 	result := v.volumes.Union(podVolumes)
-	return len(result), err
+	return len(result)
 }
 
-func (v *VolumeLimits) validate(pod *v1.Pod) (sets.String, error) {
+func (v *VolumeLimits) validate(pod *v1.Pod) sets.String {
 	podPVCs := sets.String{}
 	for _, volume := range pod.Spec.Volumes {
 		if volume.PersistentVolumeClaim != nil {
 			podPVCs.Insert(fmt.Sprintf("%s/%s", pod.Namespace, volume.PersistentVolumeClaim.ClaimName))
 		} else if volume.Ephemeral != nil {
 			// generated name per https://kubernetes.io/docs/concepts/storage/ephemeral-volumes/#persistentvolumeclaim-naming
-			pvcName := fmt.Sprintf("%s-%s", pod.Name, volume.Name)
-			podPVCs.Insert(fmt.Sprintf("%s/%s", pod.Namespace, pvcName))
+			podPVCs.Insert(fmt.Sprintf("%s/%s-%s", pod.Namespace, pod.Name, volume.Name))
 		}
 	}
 
-	return podPVCs, nil
+	return podPVCs
 }
 
 func (v *VolumeLimits) DeletePod(key types.NamespacedName) {

diff --git a/pkg/utils/options/options.go b/pkg/utils/options/options.go
@@ -45,6 +45,7 @@ func MustParse() Options {
 	flag.BoolVar(&opts.AWSENILimitedPodDensity, "aws-eni-limited-pod-density", env.WithDefaultBool("AWS_ENI_LIMITED_POD_DENSITY", true), "Indicates whether new nodes should use ENI-based pod density")
 	flag.StringVar(&opts.AWSDefaultInstanceProfile, "aws-default-instance-profile", env.WithDefaultString("AWS_DEFAULT_INSTANCE_PROFILE", ""), "The default instance profile to use when provisioning nodes in AWS")
 	flag.BoolVar(&opts.AWSEnablePodENI, "aws-enable-pod-eni", env.WithDefaultBool("AWS_ENABLE_POD_ENI", false), "If true then instances that support pod ENI will report a vpc.amazonaws.com/pod-eni resource")
+	// See https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/docs/options.md for the EBS CSI volume-attach-limit docs
 	flag.IntVar(&opts.AWSVolumeAttachLimit, "aws-volume-attach-limit", env.WithDefaultInt("AWS_VOLUME_ATTACH_LIMIT", -1), "If set to a value >=0, this is the maximum number of volumes attachable per node and should match the value passed to the EBS CSI driver.")
 	flag.Parse()
 	if err := opts.Validate(); err != nil {
@@ -67,8 +68,7 @@ type Options struct {
 	AWSENILimitedPodDensity   bool
 	AWSDefaultInstanceProfile string
 	AWSEnablePodENI           bool
-	// AWSVolumeAttachLimit is the setting of the volume-attach-limit from the ebs CSI driver
-	AWSVolumeAttachLimit int
+	AWSVolumeAttachLimit      int
 }
 
 func (o Options) Validate() (err error) {