iterative / terraform-provider-iterative

☁️ Terraform plugin for machine learning workloads: spot instance recovery & auto-termination | AWS, GCP, Azure, Kubernetes
https://registry.terraform.io/providers/iterative/iterative/latest/docs
Apache License 2.0
290 stars 27 forks source link

`task` fix pollution of environment variables for the `tpi-task.service` #561

Closed dacbd closed 2 years ago

dacbd commented 2 years ago

As discussed: the credentials used to "create" the task != credentials of the task.

See #550 comments for some additional context. Prevents #550 and #501 from working properly.

dacbd commented 2 years ago

https://github.com/iterative/terraform-provider-iterative/pull/550#issuecomment-1118889708

https://github.com/iterative/terraform-provider-iterative/blob/1df59d5c1976cada9a7f7f2fc5fb32777ce979ee/task/common/machine/script.go#L45-L47

https://github.com/iterative/terraform-provider-iterative/blob/1df59d5c1976cada9a7f7f2fc5fb32777ce979ee/task/common/machine/script.go#L130

https://github.com/iterative/terraform-provider-iterative/blob/1df59d5c1976cada9a7f7f2fc5fb32777ce979ee/task/common/machine/script.go#L13-L18

https://github.com/iterative/terraform-provider-iterative/blob/1df59d5c1976cada9a7f7f2fc5fb32777ce979ee/task/common/values.go#L85-L105

https://github.com/iterative/terraform-provider-iterative/blob/1df59d5c1976cada9a7f7f2fc5fb32777ce979ee/task/aws/resources/resource_launch_template.go#L47-L55

dacbd commented 2 years ago

Yes, the action point wouldn't be removing that logic (yet), but just making it exclusive to ExecStop*

@0x2b3bfa0 Does systemd allow different EnvironmentFiles per Exec* cmd? Or add : to the ExecStart and monkey patch the proper envs in differently?

0x2b3bfa0 commented 2 years ago

It's a bit more involved:

ExecStart should receive all the environment variables except for the credentials.

ExecStop* should receive all the environment variables.

We may need to differentiate credentials from the rest of variables and source the former only when needed.