Skip to content

Expose runner environment variables to gitlab-runner installation script #113

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

UiP9AV6Y
Copy link
Contributor

  • merge the various installation scripts into a single one
  • use text/template to render the actual script based on user/operator input
  • implement unit tests for test coverage of new feature

closes #111

@UiP9AV6Y UiP9AV6Y force-pushed the prepare-install-env branch from a00957c to 3aad91f Compare April 23, 2025 17:56
* merge the various installation scripts into a single one
* use text/template to render the actual script based on user/operator input
* implement unit tests for test coverage of new feature

closes cirruslabs#111
@UiP9AV6Y UiP9AV6Y force-pushed the prepare-install-env branch from 3aad91f to 9a3b4dd Compare April 23, 2025 17:57
@edigaryev
Copy link
Contributor

Hello Gordon,

It looks like this PR includes more changes than necessary to address #111. This makes the review process more difficult and leads to unnecessary discussions about the additional changes.

A minimal, focused diff would be greatly appreciated.

UiP9AV6Y added a commit to UiP9AV6Y/gitlab-tart-executor that referenced this pull request Apr 25, 2025
this change replaces the embedded gitlab-runner installer scripts
with a template which is rendered as a shell script based on
cli/env configuration.

the feature is inteded as preparation for cirruslabs#113
UiP9AV6Y added a commit to UiP9AV6Y/gitlab-tart-executor that referenced this pull request Apr 25, 2025
both gitlab.EnvPrefixGitLabRunner and tart.EnvPrefixTartExecutor
are public now for consistency sake. initializing the tart
configuration requires passing a key prefix, which is
usually gitlab.EnvPrefixGitLabRunner

providing the prefix ensures the gitlab and tart namespace remain
independent.

this change is in preparation for cirruslabs#113
UiP9AV6Y added a commit to UiP9AV6Y/gitlab-tart-executor that referenced this pull request Apr 25, 2025
both gitlab.EnvPrefixGitLabRunner and tart.EnvPrefixTartExecutor
are public now for consistency sake. initializing the tart
configuration requires passing a key prefix, which is
usually gitlab.EnvPrefixGitLabRunner

providing the prefix ensures the gitlab and tart namespace remain
independent.

this change is in preparation for cirruslabs#113
UiP9AV6Y added a commit to UiP9AV6Y/gitlab-tart-executor that referenced this pull request Apr 25, 2025
both gitlab.EnvPrefixGitLabRunner and tart.EnvPrefixTartExecutor
are public now for consistency sake. initializing the tart
configuration requires passing a key prefix, which is
usually gitlab.EnvPrefixGitLabRunner. providing the prefix
ensures the gitlab and tart namespace remain independent.

additionally a proxy method for os.LookupEnv has been
implemented utilizing the newly added constant as prefix
for variable lookups.

this change is in preparation for cirruslabs#113
@UiP9AV6Y
Copy link
Contributor Author

A minimal, focused diff would be greatly appreciated.

sure thing. have you identified areas which could be broken out into separate MRs? i have already prepared two changes.

@edigaryev
Copy link
Contributor

A minimal, focused diff would be greatly appreciated.

sure thing. have you identified areas which could be broken out into separate MRs? i have already prepared two changes.

Please take a look at #116.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose runner environment variables to gitlab-runner installation scripts
2 participants