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

Cost Optimizations - Quality Of Service and Config #343

Closed
10 tasks
migueltol22 opened this issue Jun 4, 2020 · 2 comments
Closed
10 tasks

Cost Optimizations - Quality Of Service and Config #343

migueltol22 opened this issue Jun 4, 2020 · 2 comments
Labels
enhancement New feature or request stale untriaged This issues has not yet been looked at by the Maintainers

Comments

@migueltol22
Copy link
Contributor

Motivation: Why do you think this is important?
As part of #342, we would like a user to be able to specify a Quality of Service Level for the workflow which corresponds to a value that will be used for queuing the workflow.

Goal: What should the final outcome look like, ideally?
A user specifies a quality of service parameter in their workflow that corresponds to a value that is part of the config set in Flyte Admin

@workflow_class(quality_of_service=0):

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Flyte component

  • Overall
  • Flyte Setup and Installation scripts
  • Flyte Documentation
  • Flyte communication (slack/email etc)
  • FlytePropeller
  • [x ] FlyteIDL (Flyte specification language)
  • [ x] Flytekit (Python SDK)
  • [ x] FlyteAdmin (Control Plane service)
  • FlytePlugins
  • DataCatalog
  • FlyteStdlib (common libraries)
  • FlyteConsole (UI)
  • Other

[Optional] Propose: Link/Inline
Change in idl and flytekit to add support for quality of service.
In FlyteAdmin these values will correspond to a time interval as part of flyteadmin config.
Something like

0: no queuing,
1: 30 mins,
2: 2 hours

Additional context
Add any other context or screenshots about the feature request here.

Is this a blocker for you to adopt Flyte
Please let us know if this makes it impossible to adopt Flyte

@migueltol22 migueltol22 added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers labels Jun 4, 2020
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022
* Start adding Vaul Secret manager

Signed-off-by: Tim Bauer <[email protected]>

* Auto-update enumer

Signed-off-by: Tim Bauer <[email protected]>

* Make verbose

Signed-off-by: Tim Bauer <[email protected]>

* Revert to print

Signed-off-by: Tim Bauer <[email protected]>

* Mark debug statements

Signed-off-by: Tim Bauer <[email protected]>

* Remove prints, simplify vault

Signed-off-by: Tim Bauer <[email protected]>

* Test format env var, print more

Signed-off-by: Tim Bauer <[email protected]>

* Check annotations

Signed-off-by: Tim Bauer <[email protected]>

* Try to retrieve annotations

Signed-off-by: Tim Bauer <[email protected]>

* Attempt append annotation

Signed-off-by: Tim Bauer <[email protected]>

* Test annotation injection

Signed-off-by: Tim Bauer <[email protected]>

* Pre-populate only

Signed-off-by: Tim Bauer <[email protected]>

* Utils func for vault secret annotation

Signed-off-by: Tim Bauer <[email protected]>

* Add shorter id to avoid 63 char limit

Signed-off-by: Tim Bauer <[email protected]>

* Rm print

Signed-off-by: Tim Bauer <[email protected]>

* Set vault role from config

Signed-off-by: Tim Bauer <[email protected]>

* Add tests

Signed-off-by: Tim Bauer <[email protected]>

* Name coreIdl import

Signed-off-by: Tim Bauer <[email protected]>

* Rm duplicate import

Signed-off-by: Tim Bauer <[email protected]>

* Update documentation and naming

Signed-off-by: Tim Bauer <[email protected]>

* Update pkg/webhook/utils.go

Co-authored-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Tim Bauer <[email protected]>

* Update pkg/webhook/vault_secret_manager.go

There is a [workaround](https://www.vaultproject.io/docs/platform/k8s/injector/examples#environment-variable-example) which involves mounting a template formatted file that contains `export API_KEY="{{ .Data.data.api_key }}"` and then sourcing this file as an extra step. But unless the user takes this extra sourcing step, this is still file mounting. So I would go with this Error message since the user should be warned that the expected result from requesting Env var will not be achieved with this.

Co-authored-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Tim Bauer <[email protected]>

* Update pkg/webhook/vault_secret_manager.go

Co-authored-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Tim Bauer <[email protected]>

* Fix naming and indent

Signed-off-by: Tim Bauer <[email protected]>

* Add handling of different kv version and test

Signed-off-by: Tim Bauer <[email protected]>

* Remove print

Signed-off-by: Tim Bauer <[email protected]>

* Add enumer for KV version

Signed-off-by: Tim Bauer <[email protected]>

* Correct kvversion type

Signed-off-by: Tim Bauer <[email protected]>

* Rm newlines from vault secret template

Signed-off-by: Tim Bauer <[email protected]>

* Apply suggestions from code review

Co-authored-by: Ketan Umare <[email protected]>
Signed-off-by: Tim Bauer <[email protected]>

* Add docstring

Signed-off-by: Tim Bauer <[email protected]>

Co-authored-by: Haytham Abuelfutuh <[email protected]>
Co-authored-by: Ketan Umare <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Jul 24, 2023
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 9, 2023
* Start adding Vaul Secret manager

Signed-off-by: Tim Bauer <[email protected]>

* Auto-update enumer

Signed-off-by: Tim Bauer <[email protected]>

* Make verbose

Signed-off-by: Tim Bauer <[email protected]>

* Revert to print

Signed-off-by: Tim Bauer <[email protected]>

* Mark debug statements

Signed-off-by: Tim Bauer <[email protected]>

* Remove prints, simplify vault

Signed-off-by: Tim Bauer <[email protected]>

* Test format env var, print more

Signed-off-by: Tim Bauer <[email protected]>

* Check annotations

Signed-off-by: Tim Bauer <[email protected]>

* Try to retrieve annotations

Signed-off-by: Tim Bauer <[email protected]>

* Attempt append annotation

Signed-off-by: Tim Bauer <[email protected]>

* Test annotation injection

Signed-off-by: Tim Bauer <[email protected]>

* Pre-populate only

Signed-off-by: Tim Bauer <[email protected]>

* Utils func for vault secret annotation

Signed-off-by: Tim Bauer <[email protected]>

* Add shorter id to avoid 63 char limit

Signed-off-by: Tim Bauer <[email protected]>

* Rm print

Signed-off-by: Tim Bauer <[email protected]>

* Set vault role from config

Signed-off-by: Tim Bauer <[email protected]>

* Add tests

Signed-off-by: Tim Bauer <[email protected]>

* Name coreIdl import

Signed-off-by: Tim Bauer <[email protected]>

* Rm duplicate import

Signed-off-by: Tim Bauer <[email protected]>

* Update documentation and naming

Signed-off-by: Tim Bauer <[email protected]>

* Update pkg/webhook/utils.go

Co-authored-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Tim Bauer <[email protected]>

* Update pkg/webhook/vault_secret_manager.go

There is a [workaround](https://www.vaultproject.io/docs/platform/k8s/injector/examples#environment-variable-example) which involves mounting a template formatted file that contains `export API_KEY="{{ .Data.data.api_key }}"` and then sourcing this file as an extra step. But unless the user takes this extra sourcing step, this is still file mounting. So I would go with this Error message since the user should be warned that the expected result from requesting Env var will not be achieved with this.

Co-authored-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Tim Bauer <[email protected]>

* Update pkg/webhook/vault_secret_manager.go

Co-authored-by: Haytham Abuelfutuh <[email protected]>
Signed-off-by: Tim Bauer <[email protected]>

* Fix naming and indent

Signed-off-by: Tim Bauer <[email protected]>

* Add handling of different kv version and test

Signed-off-by: Tim Bauer <[email protected]>

* Remove print

Signed-off-by: Tim Bauer <[email protected]>

* Add enumer for KV version

Signed-off-by: Tim Bauer <[email protected]>

* Correct kvversion type

Signed-off-by: Tim Bauer <[email protected]>

* Rm newlines from vault secret template

Signed-off-by: Tim Bauer <[email protected]>

* Apply suggestions from code review

Co-authored-by: Ketan Umare <[email protected]>
Signed-off-by: Tim Bauer <[email protected]>

* Add docstring

Signed-off-by: Tim Bauer <[email protected]>

Co-authored-by: Haytham Abuelfutuh <[email protected]>
Co-authored-by: Ketan Umare <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 21, 2023
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 21, 2023
@github-actions
Copy link

Hello 👋, This issue has been inactive for over 9 months. To help maintain a clean and focused backlog, we'll be marking this issue as stale and will close the issue if we detect no activity in the next 7 days. Thank you for your contribution and understanding! 🙏

@github-actions github-actions bot added the stale label Aug 26, 2023
@github-actions
Copy link

github-actions bot commented Sep 2, 2023

Hello 👋, This issue has been inactive for over 9 months and hasn't received any updates since it was marked as stale. We'll be closing this issue for now, but if you believe this issue is still relevant, please feel free to reopen it. Thank you for your contribution and understanding! 🙏

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 2, 2023
@eapolinario eapolinario reopened this Nov 2, 2023
@kumare3 kumare3 closed this as completed Dec 22, 2023
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Apr 30, 2024
austin362667 pushed a commit to austin362667/flyte that referenced this issue May 7, 2024
robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this issue Jul 2, 2024
troychiu pushed a commit that referenced this issue Jul 8, 2024
## Overview
Fix error handling conditional in secret injector

Without this change
```
2024/06/25 20:54:47 http: panic serving 10.232.42.27:42560: runtime error: invalid memory address or nil pointer dereference
goroutine 185 [running]:
net/http.(*conn).serve.func1()
	/usr/local/go/src/net/http/server.go:1868 +0xb9
panic({0x2bbdfc0?, 0x5423950?})
	/usr/local/go/src/runtime/panic.go:920 +0x270
github.com/flyteorg/flyte/flytepropeller/pkg/secret.EmbeddedSecretManagerInjector.Inject({{0x1, {{0x0, 0x0}}, {{0xc0007839e0, 0x18}}, {{0x31a9d0e, 0xc}, {0xc000baa030, 0xc000baa090, {...}}}}, ...}, ...)
	/go/src/github.com/unionai/cloud/flyte/flytepropeller/pkg/secret/embedded_secret_manager.go:152 +0xaa
github.com/flyteorg/flyte/flytepropeller/pkg/secret.(*SecretsPodMutator).injectSecret(0xc0015a4540, {0x38eab40, 0xc0013f44e0}, 0xc001490e40, 0x9d88c5?)
	/go/src/github.com/unionai/cloud/flyte/flytepropeller/pkg/secret/secrets_pod_mutator.go:81 +0x177
github.com/flyteorg/flyte/flytepropeller/pkg/secret.(*SecretsPodMutator).Mutate(0xc000b8e6f0?, {0x38eab40, 0xc0013f44e0}, 0xc000c00000)
	/go/src/github.com/unionai/cloud/flyte/flytepropeller/pkg/secret/secrets_pod_mutator.go:48 +0x225
```

## Test Plan
- [x] Add a unittest to cover this path

## Rollout Plan (if applicable)
Bring to cloud as part of [fasttask secrets fixes](unionai/flyte#340) that uncovered this

## Upstream Changes
Should this change be upstreamed to OSS (flyteorg/flyte)? If not, please uncheck this box, which is used for auditing. Note, it is the responsibility of each developer to actually upstream their changes. See [this guide](https://unionai.atlassian.net/wiki/spaces/ENG/pages/447610883/Flyte+-+Union+Cloud+Development+Runbook/#When-are-versions-updated%3F).
- [ ] To be upstreamed to OSS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request stale untriaged This issues has not yet been looked at by the Maintainers
Projects
None yet
Development

No branches or pull requests

3 participants