Node selector support via constraints #77

alexellis · 2017-11-08T11:05:46Z

Description

Brings parity with Docker Swarm implementation.

We can now pin a function to a specific node such as:

constraints:
  - environment=production

faas-cli YAML

How Has This Been Tested?

Test plan:

First label a node using kubectl:

$ kubectl label node/node1 environment=true

Next add a constraint to a function using the YAML example above and deploy.
You should see that the function is scheduled on the node. Now try removing the function and the constraint and schedule again.
Finally add a constraint that can never be matched and then reschedule the function, you should see it sitting at 0/1 replicas.

Next - do all of the above using the --update=true --replace=false flag to faas-cli deploy

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I've read the CONTRIBUTION guide
I have signed-off my commits with git commit -s
I have added tests to cover my changes.
All new and existing tests passed.

Signed-off-by: Alex Ellis <alexellis2@gmail.com>

alexellis · 2017-11-08T18:57:11Z

@johnmccabe @ericstoekl I'm looking to push this through. Would you be interested in getting involved testing? I know you may have multi-node clusters.

ericstoekl · 2017-11-08T19:40:02Z

Sure, I'll try this PR out. Can I add myself as a reviewer using Derek?

rgee0 · 2017-11-08T19:50:35Z

@ericstoekl you should be able to start a review when commenting in the file changes

alexellis · 2017-11-09T08:46:35Z

@rayhero I think this could be useful for your image / opencv processing

alexellis · 2017-11-09T08:47:06Z

Eric would still appreciate a review & testing.

johnmccabe · 2017-11-11T02:29:27Z

Have spun this up locally for an initial sanity check (not reviewed code yet), on a two node cluster however so I've tainted master.

My Kubernetes version is:

Server Version: version.Info{Major:"1", Minor:"8", GitVersion:"v1.8.3", GitCommit:"f0efb3cb883751c5ffdbe6d515f3cb4fbe7b7acd", GitTreeState:"clean", BuildDate:"2017-11-08T18:27:48Z", GoVersion:"go1.8.3", Compiler:"gc", Platform:"linux/amd64"}

Created the following label on kube2:

kubectl label node/kube2 environment=production

PS D:\Workspaces\go\src\github.com\openfaas\faas-netes> kubectl describe node kube2
Name:               kube2
Roles:              <none>
Labels:             beta.kubernetes.io/arch=amd64
                    beta.kubernetes.io/os=linux
                    environment=production
                    kubernetes.io/hostname=kube2

I've deployed the function both with and without constraints and then put it under a simple load (curl in a while loop).

Without constraint set
Behaves as expected with pods spread across both cluster nodes.

faas-cli deploy --image johnmccabe/fn-env --name echo --g
ateway http://kube1.johnmccabe.infra:31112

With constraint set
Behaves as expected with pods restricted to kube2

faas-cli.exe deploy --image johnmccabe/fn-env --name echo --gat
eway http://kube1.johnmccabe.infra:31112 --constraint "environment=production"

Also tried with one of the default labels.

faas-cli deploy --image johnmccabe/fn-env --name echo --g
ateway http://kube1.johnmccabe.infra:31112  --constraint "kubernetes.io/hostname=kube2"

johnmccabe

Just some minor comments, apart from that looks OK to me.

johnmccabe · 2017-11-11T02:36:50Z

handlers/deploy.go

+const WatchdogPort = 8080
+
+// InitialReplicas how many replicas to start of creating for a function
+const InitialReplicas = 1


Worth allowing the user to set the initial/min number of replicas when deploying a function?

Also does this actually need to be exported? its only used in this file.

Number 1 is out of scope for PR, but can come under the scope of the issue in the faas repo around setting max replicas. I'll un-capitalize the watchdog port until needed in another package.

johnmccabe · 2017-11-11T02:48:35Z

handlers/deploy.go

+	log.Println(constraints)
+	if len(constraints) > 0 {
+		for _, constraint := range constraints {
+			parts := strings.Split(constraint, "=")


Sanity checked with a keyonly constraint here and it looked ok.

faas-cli.exe deploy --image johnmccabe/fn-env --name echo --gateway http://kube1.johnmccabe.infra:31112 --constraint "keyonlylabel="

johnmccabe · 2017-11-11T02:54:08Z

handlers/deploy.go

@@ -22,6 +23,12 @@ import (
 	"k8s.io/client-go/kubernetes"
 )

+// WatchdogPort for the OpenFaaS function watchdog
+const WatchdogPort = 8080


Does this actually need to be exported? its only used in this file.

Perhaps pull this in from a package either in the watchdog or gateway so it gets defined in one location? (Was wondering if we'd ever need to change this.. say a user is packaging up a binary they can't change which binds to 8080.. bit of a contrived edge case tho so not worth it until its seen in the wild).

johnmccabe · 2017-11-11T16:20:39Z

@alexellis can this move to done in the project

alexellis · 2017-11-11T16:21:36Z

Do you have access?

alexellis · 2017-11-11T23:51:40Z

Thanks for taking time to test and validate

The Kubernetes operator `faas-netes` expects the constraints / nodeSelectors in the format <label-key>=<label-value>. References: - Pull Request ([Node selector support via constraints openfaas#77](openfaas/faas-netes#77)), which introduced the feature - the [source code for the createSelector function](https://github.com/openfaas/faas-netes/blame/master/pkg/handlers/deploy.go#L338), see how the key-value pair is split - an [issue](openfaas/faas-netes#406 (comment)) regarding the difference between constraints for Docker Swarm & Kubernetes also remove some erroneous whitespace in an URL, which broke the link and anchor

The Kubernetes operator `faas-netes` expects the constraints / nodeSelectors in the format <label-key>=<label-value>. references: - pull request ([Node selector support via constraints openfaas#77](openfaas/faas-netes#77)), which introduced the feature - the [source code for the createSelector function](https://github.com/openfaas/faas-netes/blame/master/pkg/handlers/deploy.go#L338), see how the key-value pair is split - an [issue](openfaas/faas-netes#406 (comment)) regarding the difference between constraints for Docker Swarm & Kubernetes Signed-off-by: Robert Groh <robert.groh@amity.group>

The Kubernetes operator `faas-netes` expects the constraints / nodeSelectors in the format <label-key>=<label-value>. references: - pull request ([Node selector support via constraints openfaas#77](openfaas/faas-netes#77)), which introduced the feature - the [source code for the createSelector function](https://github.com/openfaas/faas-netes/blame/master/pkg/handlers/deploy.go#L338), see how the key-value pair is split - an [issue](openfaas/faas-netes#406 (comment)) regarding the difference between constraints for Docker Swarm & Kubernetes also: - fix broken link/anchor by removing erroneous whitespace in the URL Signed-off-by: Robert Groh <robert.groh@amity.group>

Add constraint support via nodeSelector API

a70689d

Signed-off-by: Alex Ellis <alexellis2@gmail.com>

alexellis force-pushed the nodeSelector branch from 7fdd0a9 to a70689d Compare November 8, 2017 11:07

alexellis mentioned this pull request Nov 8, 2017

Question: Is there a way to restrict to which nodes a function can be deployed? openfaas/faas#373

Closed

alexellis added this to In review in #TeamServerless Nov 8, 2017

alexellis requested a review from ericstoekl November 8, 2017 22:39

alexellis self-assigned this Nov 8, 2017

alexellis added the priority/high label Nov 8, 2017

alexellis merged commit f3919c7 into master Nov 9, 2017

johnmccabe reviewed Nov 11, 2017

View reviewed changes

ericstoekl mentioned this pull request Nov 11, 2017

faas-cli deploy needs to reset constraints array after each function deploy in stack YAML openfaas/faas-cli#218

Closed

alexellis moved this from In review to Read to merge in #TeamServerless Nov 11, 2017

alexellis moved this from Read to merge to Done in #TeamServerless Nov 11, 2017

alexellis deleted the nodeSelector branch November 11, 2017 23:51

alexellis removed this from Done in #TeamServerless Mar 15, 2018

robertgroh mentioned this pull request Mar 12, 2021

add note for kubernetes constraints/nodeSelectors openfaas/docs#260

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node selector support via constraints #77

Node selector support via constraints #77

alexellis commented Nov 8, 2017 •

edited

Loading

alexellis commented Nov 8, 2017

ericstoekl commented Nov 8, 2017

rgee0 commented Nov 8, 2017

alexellis commented Nov 9, 2017

alexellis commented Nov 9, 2017

johnmccabe commented Nov 11, 2017 •

edited

Loading

johnmccabe left a comment

johnmccabe Nov 11, 2017

alexellis Nov 11, 2017 •

edited

Loading

johnmccabe Nov 11, 2017

johnmccabe Nov 11, 2017 •

edited

Loading

johnmccabe commented Nov 11, 2017

alexellis commented Nov 11, 2017

alexellis commented Nov 11, 2017

Node selector support via constraints #77

Node selector support via constraints #77

Conversation

alexellis commented Nov 8, 2017 • edited Loading

Description

How Has This Been Tested?

Test plan:

Types of changes

Checklist:

alexellis commented Nov 8, 2017

ericstoekl commented Nov 8, 2017

rgee0 commented Nov 8, 2017

alexellis commented Nov 9, 2017

alexellis commented Nov 9, 2017

johnmccabe commented Nov 11, 2017 • edited Loading

johnmccabe left a comment

Choose a reason for hiding this comment

johnmccabe Nov 11, 2017

Choose a reason for hiding this comment

alexellis Nov 11, 2017 • edited Loading

Choose a reason for hiding this comment

johnmccabe Nov 11, 2017

Choose a reason for hiding this comment

johnmccabe Nov 11, 2017 • edited Loading

Choose a reason for hiding this comment

johnmccabe commented Nov 11, 2017

alexellis commented Nov 11, 2017

alexellis commented Nov 11, 2017

alexellis commented Nov 8, 2017 •

edited

Loading

johnmccabe commented Nov 11, 2017 •

edited

Loading

alexellis Nov 11, 2017 •

edited

Loading

johnmccabe Nov 11, 2017 •

edited

Loading