Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tolerations for extended resources #6033

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ var (
DefaultPodTemplateResync: config2.Duration{
Duration: 30 * time.Second,
},
UpdateBaseBackoffDuration: 10,
UpdateBackoffRetries: 5,
UpdateBaseBackoffDuration: 10,
UpdateBackoffRetries: 5,
AddTolerationsForExtendedResources: []string{},
}

// K8sPluginConfigSection provides a singular top level config section for all plugins.
Expand Down Expand Up @@ -214,6 +215,9 @@ type K8sPluginConfig struct {

// Number of retries for exponential backoff when updating a resource.
UpdateBackoffRetries int `json:"update-backoff-retries" pflag:",Number of retries for exponential backoff when updating a resource."`

// Extended resources that should be added to the tolerations automatically.
AddTolerationsForExtendedResources []string `json:"add-tolerations-for-extended-resources" pflag:",Name of the extended resources for which tolerations should be added."`
}

// FlyteCoPilotConfig specifies configuration for the Flyte CoPilot system. FlyteCoPilot, allows running flytekit-less containers
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 51 additions & 0 deletions flyteplugins/go/tasks/pluginmachinery/flytek8s/pod_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"github.com/imdario/mergo"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
pluginserrors "github.com/flyteorg/flyte/flyteplugins/go/tasks/errors"
Expand Down Expand Up @@ -445,6 +446,54 @@
}
}

func addTolerationInPodSpec(podSpec *v1.PodSpec, toleration *v1.Toleration) *v1.PodSpec {
podTolerations := podSpec.Tolerations

var newTolerations []v1.Toleration
for i := range podTolerations {
if toleration.MatchToleration(&podTolerations[i]) {
return podSpec
}
newTolerations = append(newTolerations, podTolerations[i])
}
newTolerations = append(newTolerations, *toleration)
podSpec.Tolerations = newTolerations
return podSpec
}

func AddTolerationsForExtendedResources(podSpec *v1.PodSpec) *v1.PodSpec {
if podSpec == nil {
podSpec = &v1.PodSpec{}
}

Check warning on line 467 in flyteplugins/go/tasks/pluginmachinery/flytek8s/pod_helper.go

View check run for this annotation

Codecov / codecov/patch

flyteplugins/go/tasks/pluginmachinery/flytek8s/pod_helper.go#L466-L467

Added lines #L466 - L467 were not covered by tests

resources := sets.NewString()
for _, container := range podSpec.Containers {
for _, extendedResource := range config.GetK8sPluginConfig().AddTolerationsForExtendedResources {
if _, ok := container.Resources.Requests[v1.ResourceName(extendedResource)]; ok {
resources.Insert(extendedResource)
}
}
}

for _, container := range podSpec.InitContainers {
for _, extendedResource := range config.GetK8sPluginConfig().AddTolerationsForExtendedResources {
if _, ok := container.Resources.Requests[v1.ResourceName(extendedResource)]; ok {
resources.Insert(extendedResource)
}
}
}

for _, resource := range resources.List() {
addTolerationInPodSpec(podSpec, &v1.Toleration{
Key: resource,
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoSchedule,
})
}

return podSpec
}

// ToK8sPodSpec builds a PodSpec and ObjectMeta based on the definition passed by the TaskExecutionContext. This
// involves parsing the raw PodSpec definition and applying all Flyte configuration options.
func ToK8sPodSpec(ctx context.Context, tCtx pluginsCore.TaskExecutionContext) (*v1.PodSpec, *metav1.ObjectMeta, string, error) {
Expand All @@ -460,6 +509,8 @@
return nil, nil, "", err
}

podSpec = AddTolerationsForExtendedResources(podSpec)

return podSpec, objectMeta, primaryContainerName, nil
}

Expand Down
110 changes: 110 additions & 0 deletions flyteplugins/go/tasks/pluginmachinery/flytek8s/pod_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package flytek8s
import (
"context"
"encoding/json"
"fmt"
"io/ioutil"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -2244,3 +2245,112 @@ func TestAddFlyteCustomizationsToContainer_SetConsoleUrl(t *testing.T) {
})
}
}

func TestAddTolerationsForExtendedResources(t *testing.T) {
gpuResourceName := v1.ResourceName("nvidia.com/gpu")
addTolerationResourceName := v1.ResourceName("foo/bar")
noTolerationResourceName := v1.ResourceName("foo/baz")
assert.NoError(t, config.SetK8sPluginConfig(&config.K8sPluginConfig{
GpuResourceName: gpuResourceName,
AddTolerationsForExtendedResources: []string{
gpuResourceName.String(),
addTolerationResourceName.String(),
},
}))

podSpec := &v1.PodSpec{
Containers: []v1.Container{
v1.Container{
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
gpuResourceName: resource.MustParse("1"),
addTolerationResourceName: resource.MustParse("1"),
noTolerationResourceName: resource.MustParse("1"),
},
},
},
},
Tolerations: []v1.Toleration{
{
Key: "foo",
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoSchedule,
},
},
}

podSpec = AddTolerationsForExtendedResources(podSpec)
fmt.Printf("%v\n", podSpec.Tolerations)
assert.Equal(t, 3, len(podSpec.Tolerations))
assert.Equal(t, addTolerationResourceName.String(), podSpec.Tolerations[1].Key)
assert.Equal(t, v1.TolerationOpExists, podSpec.Tolerations[1].Operator)
assert.Equal(t, v1.TaintEffectNoSchedule, podSpec.Tolerations[1].Effect)
assert.Equal(t, gpuResourceName.String(), podSpec.Tolerations[2].Key)
assert.Equal(t, v1.TolerationOpExists, podSpec.Tolerations[2].Operator)
assert.Equal(t, v1.TaintEffectNoSchedule, podSpec.Tolerations[2].Effect)

podSpec = &v1.PodSpec{
InitContainers: []v1.Container{
v1.Container{
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
gpuResourceName: resource.MustParse("1"),
addTolerationResourceName: resource.MustParse("1"),
noTolerationResourceName: resource.MustParse("1"),
},
},
},
},
Tolerations: []v1.Toleration{
{
Key: "foo",
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoSchedule,
},
},
}

podSpec = AddTolerationsForExtendedResources(podSpec)
assert.Equal(t, 3, len(podSpec.Tolerations))
assert.Equal(t, addTolerationResourceName.String(), podSpec.Tolerations[1].Key)
assert.Equal(t, v1.TolerationOpExists, podSpec.Tolerations[1].Operator)
assert.Equal(t, v1.TaintEffectNoSchedule, podSpec.Tolerations[1].Effect)
assert.Equal(t, gpuResourceName.String(), podSpec.Tolerations[2].Key)
assert.Equal(t, v1.TolerationOpExists, podSpec.Tolerations[2].Operator)
assert.Equal(t, v1.TaintEffectNoSchedule, podSpec.Tolerations[2].Effect)

podSpec = &v1.PodSpec{
Containers: []v1.Container{
v1.Container{
Resources: v1.ResourceRequirements{
Requests: v1.ResourceList{
gpuResourceName: resource.MustParse("1"),
addTolerationResourceName: resource.MustParse("1"),
noTolerationResourceName: resource.MustParse("1"),
},
},
},
},
Tolerations: []v1.Toleration{
{
Key: "foo",
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoSchedule,
},
{
Key: gpuResourceName.String(),
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoSchedule,
},
},
}

podSpec = AddTolerationsForExtendedResources(podSpec)
assert.Equal(t, 3, len(podSpec.Tolerations))
assert.Equal(t, gpuResourceName.String(), podSpec.Tolerations[1].Key)
assert.Equal(t, v1.TolerationOpExists, podSpec.Tolerations[1].Operator)
assert.Equal(t, v1.TaintEffectNoSchedule, podSpec.Tolerations[1].Effect)
assert.Equal(t, addTolerationResourceName.String(), podSpec.Tolerations[2].Key)
assert.Equal(t, v1.TolerationOpExists, podSpec.Tolerations[2].Operator)
assert.Equal(t, v1.TaintEffectNoSchedule, podSpec.Tolerations[2].Effect)
}
17 changes: 14 additions & 3 deletions flyteplugins/go/tasks/plugins/k8s/dask/dask_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -525,9 +525,10 @@ func TestBuildResouceDaskUsePodTemplate(t *testing.T) {

func TestBuildResourceDaskExtendedResources(t *testing.T) {
assert.NoError(t, config.SetK8sPluginConfig(&config.K8sPluginConfig{
GpuDeviceNodeLabel: "gpu-node-label",
GpuPartitionSizeNodeLabel: "gpu-partition-size",
GpuResourceName: flytek8s.ResourceNvidiaGPU,
GpuDeviceNodeLabel: "gpu-node-label",
GpuPartitionSizeNodeLabel: "gpu-partition-size",
GpuResourceName: flytek8s.ResourceNvidiaGPU,
AddTolerationsForExtendedResources: []string{"nvidia.com/gpu"},
}))

fixtures := []struct {
Expand Down Expand Up @@ -569,6 +570,11 @@ func TestBuildResourceDaskExtendedResources(t *testing.T) {
Operator: v1.TolerationOpEqual,
Effect: v1.TaintEffectNoSchedule,
},
{
Key: "nvidia.com/gpu",
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoSchedule,
},
},
},
{
Expand Down Expand Up @@ -620,6 +626,11 @@ func TestBuildResourceDaskExtendedResources(t *testing.T) {
Operator: v1.TolerationOpEqual,
Effect: v1.TaintEffectNoSchedule,
},
{
Key: "nvidia.com/gpu",
Operator: v1.TolerationOpExists,
Effect: v1.TaintEffectNoSchedule,
},
},
},
}
Expand Down
17 changes: 14 additions & 3 deletions flyteplugins/go/tasks/plugins/k8s/kfoperators/mpi/mpi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,10 @@ func TestBuildResourceMPIForWrongInput(t *testing.T) {

func TestBuildResourceMPIExtendedResources(t *testing.T) {
assert.NoError(t, flytek8sConfig.SetK8sPluginConfig(&flytek8sConfig.K8sPluginConfig{
GpuDeviceNodeLabel: "gpu-node-label",
GpuPartitionSizeNodeLabel: "gpu-partition-size",
GpuResourceName: flytek8s.ResourceNvidiaGPU,
GpuDeviceNodeLabel: "gpu-node-label",
GpuPartitionSizeNodeLabel: "gpu-partition-size",
GpuResourceName: flytek8s.ResourceNvidiaGPU,
AddTolerationsForExtendedResources: []string{"nvidia.com/gpu"},
}))

fixtures := []struct {
Expand Down Expand Up @@ -412,6 +413,11 @@ func TestBuildResourceMPIExtendedResources(t *testing.T) {
Operator: corev1.TolerationOpEqual,
Effect: corev1.TaintEffectNoSchedule,
},
{
Key: "nvidia.com/gpu",
Operator: corev1.TolerationOpExists,
Effect: corev1.TaintEffectNoSchedule,
},
},
},
{
Expand Down Expand Up @@ -463,6 +469,11 @@ func TestBuildResourceMPIExtendedResources(t *testing.T) {
Operator: corev1.TolerationOpEqual,
Effect: corev1.TaintEffectNoSchedule,
},
{
Key: "nvidia.com/gpu",
Operator: corev1.TolerationOpExists,
Effect: corev1.TaintEffectNoSchedule,
},
},
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,9 +473,10 @@ func TestBuildResourcePytorchContainerImage(t *testing.T) {

func TestBuildResourcePytorchExtendedResources(t *testing.T) {
assert.NoError(t, flytek8sConfig.SetK8sPluginConfig(&flytek8sConfig.K8sPluginConfig{
GpuDeviceNodeLabel: "gpu-node-label",
GpuPartitionSizeNodeLabel: "gpu-partition-size",
GpuResourceName: flytek8s.ResourceNvidiaGPU,
GpuDeviceNodeLabel: "gpu-node-label",
GpuPartitionSizeNodeLabel: "gpu-partition-size",
GpuResourceName: flytek8s.ResourceNvidiaGPU,
AddTolerationsForExtendedResources: []string{"nvidia.com/gpu"},
}))

fixtures := []struct {
Expand Down Expand Up @@ -517,6 +518,11 @@ func TestBuildResourcePytorchExtendedResources(t *testing.T) {
Operator: corev1.TolerationOpEqual,
Effect: corev1.TaintEffectNoSchedule,
},
{
Key: "nvidia.com/gpu",
Operator: corev1.TolerationOpExists,
Effect: corev1.TaintEffectNoSchedule,
},
},
},
{
Expand Down Expand Up @@ -568,6 +574,11 @@ func TestBuildResourcePytorchExtendedResources(t *testing.T) {
Operator: corev1.TolerationOpEqual,
Effect: corev1.TaintEffectNoSchedule,
},
{
Key: "nvidia.com/gpu",
Operator: corev1.TolerationOpExists,
Effect: corev1.TaintEffectNoSchedule,
},
},
},
}
Expand Down
4 changes: 4 additions & 0 deletions flyteplugins/go/tasks/plugins/k8s/ray/ray.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,8 @@ func buildHeadPodTemplate(primaryContainer *v1.Container, basePodSpec *v1.PodSpe
return v1.PodTemplateSpec{}, err
}

basePodSpec = flytek8s.AddTolerationsForExtendedResources(basePodSpec)

podTemplateSpec := v1.PodTemplateSpec{
Spec: *basePodSpec,
ObjectMeta: *objectMeta,
Expand Down Expand Up @@ -502,6 +504,8 @@ func buildWorkerPodTemplate(primaryContainer *v1.Container, basePodSpec *v1.PodS
return v1.PodTemplateSpec{}, err
}

basePodSpec = flytek8s.AddTolerationsForExtendedResources(basePodSpec)

podTemplateSpec := v1.PodTemplateSpec{
Spec: *basePodSpec,
ObjectMeta: *objectMetadata,
Expand Down
Loading
Loading