search

cirruslabs / gitlab-tart-executor

GitLab Runner executor to run jobs in Tart VMs
MIT License
60 stars 5 forks source link

Switch from command-line arguments to environment variables where possible #12

Closed edigaryev closed 1 year ago

edigaryev commented 1 year ago

Problem 1: it's possible to specify VM image on a per-job basis, but no way to specify SSH username, password and other VM-specific settings in the same way because Tart executor is configured only once via command-line arguments for many job invocations.

Solution: switch to environment variables where possible, as done in https://github.com/cirruslabs/gitlab-tart-executor/pull/11.

Problem 2: it might be confusing for the user to figure out which stages accept which command-line arguments.

Solution: switch to environment variables where possible, only use command-line arguments (1) for global settings affecting multiple job runs and (2) in config stage, which has an ability to pass down variables for other stages to use.

Problem 3: per-job CPU/memory settings configured via environment variables may conflict with the auto resources feature when introduced.

Solution: actually introduce a config-stage command-line argument and make Tart executor ignore (or alternatively, log a warning/throw an error) the CPU/memory definitions set via the environment variables.

fkorotkov commented 1 year ago

I've added numbers to the problems so it easier to refer.

I agree with problem 1. Credentials for SSH, network configuration and probably shell configuration (#8) should be configured via environment variables because they can change on image/job basis.

I have concerns regarding problems 2 and 3. For me it's more confusing to try to configure CPU/memory via environment variables since I might not know concurrency configuration of a worker. IMO such things should be handled by having multiple runners with static configuration but with different tags that user can specify in their jobs.

Until we have more feedback from users on problem 2 and 3 I propose to only make the SSH credentials, network and shell being configured via environment variables. What do you think?

edigaryev commented 1 year ago

What do you think?

OK, but should we still try to add these CPU/memory command-line arguments only to a config sub-command to solve the problem 2?

fkorotkov commented 1 year ago

I think CPU/Memory is fine on the run stage. It's where we clone a VM and change defaults.

edigaryev commented 1 year ago

I think CPU/Memory is fine on the run stage. It's where we clone a VM and change defaults.

That's actually what I was trying to prevent in problem 2's solution: the cloning and VM settings configuration happens on prepare stage, not on run stage. And even if we get confused sometimes knowing the code, our users will certainly do more frequently.

Changed in https://github.com/cirruslabs/gitlab-tart-executor/pull/11/commits/80c93816e14fe77a33b5fdf534114218665b8e9a.