Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Replace lower-cased sidecar gpu resource with nvidia one #65

Merged
merged 2 commits into from
Mar 9, 2020

Conversation

katrogan
Copy link
Contributor

@katrogan katrogan commented Mar 7, 2020

Addresses flyteorg/flyte#180 by substituting "gpu" from the sidecar custom resources with the fully nvdia gpu resource name.

When we build Flyte container tasks we never even use "GPU", but rather the full nvidia name (https://github.com/lyft/flytepropeller/blob/f487cfe6e4d35b52f6f9cb399cceb5abf4a0595a/pkg/utils/k8s.go#L58)

I believe only sidecar tasks are (incorrectly) using the "GPU" search string in flyteplugins

wild-endeavor
wild-endeavor previously approved these changes Mar 7, 2020
Copy link
Contributor

@wild-endeavor wild-endeavor left a comment

Choose a reason for hiding this comment

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

add a description and link to the other PRs?

@katrogan
Copy link
Contributor Author

katrogan commented Mar 9, 2020

add a description and link to the other PRs?

@wild-endeavor done

@katrogan katrogan requested a review from kumare3 March 9, 2020 17:44
@@ -27,13 +27,16 @@
}],
"resources": {
"requests": {
"gpu": {
Copy link
Contributor

Choose a reason for hiding this comment

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

so the point of this test is that regardless of whether you specify gpu or the nvidia gpu, both work?

@katrogan katrogan merged commit e79dce8 into master Mar 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants