Skip to content

Conversation

@aramase
Copy link
Member

@aramase aramase commented Dec 13, 2021

Signed-off-by: Anish Ramasekar [email protected]

  • Adds Dockerfile
  • Adds build targets for the container
  • Implements the filter and score plugins
    • Use PreFilter and PreScore to perform the required computation one time and annotate the pod with the node preference and placement policy used.
    • The node preference annotation is used by Filter and Score to make a decision for the node

@aramase aramase requested review from helayoty and khenidak December 13, 2021 23:30

type PlacementPolicyPodInfos map[types.UID]*PlacementPolicyPodInfo

type PlacementPolicyPodInfo struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use cache instead of adding annotation and create a new addEventHandler from podInformer and update the cache with the unassigned pods. An example can be found here capacity scheduling.

}

// nodeWithMatchingLabels is a group of nodes that have the same labels as defined in the placement policy
nodeWithMatchingLabels := groupNodesWithLabels(nodeList, pp.Spec.NodeSelector.MatchLabels)
Copy link
Collaborator

@helayoty helayoty Dec 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we introduce an internal counter let's call it currentTargetSize and update it everytime we assign a pod to a node?

Then filter the pods with the pp.PodSelector from (nodeList).Get(i).NodeInfo.Requested to know how many pods have already been scheduled and compare the number with the currentTargetSize, wdyt?

I'm trying to get rid of annotating every pod we will schedule.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, having the annotation on every single is more deterministic because we are annotating the required action on it. The annotation indicates where the plugin wanted the pod to end up and we can use to validate if that did get honored. Even if we maintain the internal counter, we're still making the same set of API calls to construct the state?

Copy link
Collaborator

@helayoty helayoty Dec 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get the value of adding annotations for observability reasons but I am opposed to depending on them in the scheduler core logic.
Also, trying to optimize the query we are asking apiServer to perform has +ve weight. Especially if we have a large cluster with many nodes and thousand of pods.

@helayoty helayoty linked an issue Dec 16, 2021 that may be closed by this pull request
@aramase aramase marked this pull request as ready for review December 17, 2021 17:57
@aramase aramase requested a review from helayoty December 17, 2021 18:22
Copy link
Collaborator

@helayoty helayoty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we agreed, we will have this PR merged to be able to continue the integration test and @aramase with open an issue with the opened comments to discuss it later with @khenidak

Copy link
Member Author

@aramase aramase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging this PR with #25 so we have core logic to use with integration and e2e tests.

@aramase aramase merged commit 076a4b8 into Azure:main Dec 18, 2021
@aramase aramase deleted the plugin branch December 18, 2021 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add dockerfile and docker build to Makefile

2 participants