refactor!: rename variables and prefix with `runner_manager`, `runner`, `runner_worker` and global scope #757

kayman-mk · 2023-03-20T20:12:23Z

Description

Moves all variables to a new block:

runner_manager in case it configures the "main" process which sets the defaults for all runners
runner in case it configures the runner created by the runner manager
runner_worker in case it configures the docker/docker+machine
or leave it as it is, if it is a global scope, e.g. common tags, the environment, ...

Closes #575

Migrations required

Yes. Use the provided script to migrate to version 7.0

Verification

Please mention the examples you have verified.

## Description Removes the earlier deprecated `runners_pull_policy` variable. Since were making a Major release I thought this one was nice to catch. ## Migrations required YES. Replace the `runners_pull_policy` by `runners_pull_policies`.

## Description This PR removes all variables which are marked as deprecated. - `arn_format` - `subnet_id_runners` - `subnet_ids_gitlab_runner` - `asg_terminate_lifecycle_hook_create` - `asg_terminate_lifecycle_hook_heartbeat_timeout` - `asg_terminate_lifecycle_lambda_memory_size` - `asg_terminate_lifecycle_lambda_runtime` - `asg_terminate_lifecycle_lambda_timeout` ## Migrations required Yes. Remove the variables from your configuration. This is done automatically by the migration script. ## Verification None. --------- Co-authored-by: Tyrone Meijn <tyrone_meijn@hotmail.com>

github-actions · 2023-03-20T20:12:52Z

Hey @kayman-mk! 👋

Thank you for your contribution to the project. Please refer to the contribution rules for a quick overview of the process.

Make sure that this PR clearly explains:

the problem being solved
the best way a reviewer and you can test your changes

With submitting this PR you confirm that you hold the rights of the code added and agree that it will published under this LICENSE.

The following ChatOps commands are supported:

/help: notifies a maintainer to help you out

Simply add a comment with the command in the first line. If you need to pass more information, separate it with a blank line from the command.

This message was generated automatically. You are welcome to improve it.

github-actions · 2023-03-20T20:19:42Z

🦙 MegaLinter status: ❌ ERROR

Descriptor	Linter	Files	Fixed	Errors	Elapsed time
✅ ACTION	actionlint	1		0	0.05s
✅ BASH	bash-exec	1		0	0.0s
✅ BASH	shellcheck	1		0	0.1s
✅ BASH	shfmt	1	1	0	0.02s
✅ COPYPASTE	jscpd	yes		no	0.92s
✅ JSON	eslint-plugin-jsonc	1	0	0	0.95s
✅ JSON	jsonlint	1		0	0.14s
✅ JSON	prettier	1	0	0	0.43s
✅ JSON	v8r	1		0	2.64s
⚠️ MARKDOWN	markdownlint	4	4	4	0.9s
✅ MARKDOWN	markdown-link-check	4		0	7.91s
✅ REPOSITORY	checkov	yes		no	13.32s
✅ REPOSITORY	dustilock	yes		no	0.16s
✅ REPOSITORY	gitleaks	yes		no	0.68s
✅ REPOSITORY	git_diff	yes		no	0.01s
✅ REPOSITORY	secretlint	yes		no	0.86s
✅ REPOSITORY	syft	yes		no	0.29s
❌ SPELL	cspell	25		3	5.39s
✅ TERRAFORM	checkov	16		0	39.48s
✅ TERRAFORM	terraform-fmt	16	1	0	1.02s
❌ TERRAFORM	terrascan	yes		1	3.67s
✅ YAML	prettier	1	1	0	0.46s
✅ YAML	v8r	1		0	1.56s
✅ YAML	yamllint	1		0	0.22s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by

kayman-mk · 2023-03-20T21:58:37Z

Did some experiments today. I think, it is not a good idea to use blocks.

They look good and structure the variables into groups. Blocks can be grouped in other blocks, which gives a good strucuture for the variables.

But the drawback is, that I can't document these variables in a way to get support from my IDE.

So I will go with the following:

module "gitlab_runner" {
 agent_var_1 = 1
 agent_var_2 = 2

 executor_var_ 1 = 1
}

instead of

module "gitlab_runner" {
 agent {
  var_1 = 1
  var_2 = 2
 }

 executor {
  var_1 = 1
 }
}

tmeijn · 2023-03-21T10:10:57Z

I'm a bit confused...

module "gitlab_runner" {
 agent {
  var_1 = 1
  var_2 = 2
 }

 executor {
  var_1 = 1
 }
}

AFAIK this syntax is not possible for modules and can only be defined for provider resources and datasources.

kayman-mk · 2023-03-21T22:49:57Z

You are right. There is a = missing, agent = {...}. But anyway, I will not use it due to the documentation problems.

## Description Removes the earlier deprecated `runners_pull_policy` variable. Since were making a Major release I thought this one was nice to catch. ## Migrations required YES. Replace the `runners_pull_policy` by `runners_pull_policies`.

## Description This PR removes all variables which are marked as deprecated. - `arn_format` - `subnet_id_runners` - `subnet_ids_gitlab_runner` - `asg_terminate_lifecycle_hook_create` - `asg_terminate_lifecycle_hook_heartbeat_timeout` - `asg_terminate_lifecycle_lambda_memory_size` - `asg_terminate_lifecycle_lambda_runtime` - `asg_terminate_lifecycle_lambda_timeout` ## Migrations required Yes. Remove the variables from your configuration. This is done automatically by the migration script. ## Verification None. --------- Co-authored-by: Tyrone Meijn <tyrone_meijn@hotmail.com>

tmeijn · 2023-04-05T17:37:59Z

FYI: https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/3996/diffs#note_1342739227 and https://gitlab.com/gitlab-org/gitlab/-/issues/398139

## Description Removes the earlier deprecated `runners_pull_policy` variable. Since were making a Major release I thought this one was nice to catch. ## Migrations required YES. Replace the `runners_pull_policy` by `runners_pull_policies`.

This PR removes all variables which are marked as deprecated. - `arn_format` - `subnet_id_runners` - `subnet_ids_gitlab_runner` - `asg_terminate_lifecycle_hook_create` - `asg_terminate_lifecycle_hook_heartbeat_timeout` - `asg_terminate_lifecycle_lambda_memory_size` - `asg_terminate_lifecycle_lambda_runtime` - `asg_terminate_lifecycle_lambda_timeout` Yes. Remove the variables from your configuration. This is done automatically by the migration script. None. --------- Co-authored-by: Tyrone Meijn <tyrone_meijn@hotmail.com>

## Description Removes the earlier deprecated `runners_pull_policy` variable. Since were making a Major release I thought this one was nice to catch. ## Migrations required YES. Replace the `runners_pull_policy` by `runners_pull_policies`.

kayman-mk · 2023-04-20T07:51:33Z

@tmeijn Counted 126 variables in the module so far. I will follow your suggestion and group them into a map despite the documentation issues I had. I think it is worth to reference the GitLab/Docker Machine documentation and not to repeat everything here again.

## Description Adds a new variable `runners_docker_options` which holds all values for the `[runners.docker]` section and makes the single variables - `runners_image` - `runners_privileged` - `runners_disable_cache` - `runners_additional_volumes` - `runners_shm_size` - `runners_docker_runtime` - `runners_helper_image` - `runners_pull_policy` obsolete. ## Migrations required Yes, as the minimum Terraform version is 1.3.0 to support optional block variables with defaults. A migration script is provided to restructure the variables. See `/migrations/migrate-to-7-0-0.sh`. Attention Mac users: The script will not work out of the box as the `sed` implementation is different. Use a Docker container with Alpine or Ubuntu to run the script. ```hcl module "gitlab_ci_runner" { ... runners_docker_options { # set whatever is necessary } ``` ## Verification - [x] Use current configuration and ensure that the `config.toml` remains unchanged - [x] Set all new block variables and ensure that the `config.toml` is valid (use `gitlab-runner verify) - [x] Check that the default settings with Terraform 1.3 work as expected - [x] Verify all docker settings against the documentation to ensure correct names The runner starts in both cases and is available in Gitlab. No example tested but used our active configuration at Hapag-Lloyd. --------- Co-authored-by: Tyrone Meijn <tyrone_meijn@hotmail.com>

kayman-mk · 2023-04-20T10:15:31Z

@npalm This one is ready now and will be merged into refactor-variables first. All variables have been renamed and reordered according to the PR description. Thanks to @tmeijn for providing a glossary link to GitLab.

I will further group variables were it makes sense as we have 118 now. Grouping into objects helps everyone to stay on top, I think. But I create a separate PR for this.

kayman-mk · 2023-04-27T08:27:20Z

Closed in favor of #810 which includes the renaming and restructuring.

tmeijn and others added 3 commits March 16, 2023 08:57

add note for variable section

7cc41da

kayman-mk changed the base branch from main to refactor-variables March 20, 2023 20:12

kayman-mk changed the title ~~refactor!: move variables into agent, executor and global scope~~ refactor!: rename variables and prefix with agent, executor and global scope Mar 21, 2023

kayman-mk and others added 12 commits March 22, 2023 20:23

remove aws_region

d7ad4fe

rename global variables

df68fb0

refactor some variables

ded5a63

refactor some variables

b4db8bf

refactor some more

b1e90d7

refactor some more variables

a51a42b

more variables

ba7af60

refactor remaining

d0e2729

fix syntax error

dd58916

format code

05345cf

kayman-mk force-pushed the refactor-variables branch from 3fdd3c5 to c78907a Compare March 23, 2023 14:40

Merge branch 'refactor-variables' into kayma/split-variables

0165e8d

tmeijn and others added 2 commits April 20, 2023 08:13

kayman-mk force-pushed the refactor-variables branch from c78907a to 93657e6 Compare April 20, 2023 06:17

kayman-mk and others added 2 commits April 20, 2023 08:36

merge refactor-variables

74d978e

kayman-mk force-pushed the refactor-variables branch from babc50a to 3662eeb Compare April 20, 2023 07:22

merge refactor-variables

8aded23

kayman-mk and others added 13 commits April 20, 2023 09:54

fix syntax error

13874f1

merge refactor-variables

fe78e96

fix wrong variable name

e324a7f

fix syntax error

30f03a4

fix lint issues

90a9c48

fix syntax error

c13cac4

removed obsolete variables present in executor_docker_options now

b5f9e17

rename variables agent_* to runner_manager_*

2d79ffc

rename variables in script too

e014dc6

rename variables executor_* to runner_worker_*

a94a9e6

identify runner_manager variables

31664a0

split variables in runner_manager and runner

3a76dc7

kayman-mk changed the title ~~refactor!: rename variables and prefix with agent, executor and global scope~~ refactor!: rename variables and prefix with runner_manager, runner, runner_worker and global scope Apr 20, 2023

kayman-mk added 4 commits April 20, 2023 12:00

format code

ea2401d

fix syntax error

71b4f9f

fix syntax error

8887fb0

fix wrong variable name in conversion

5f507a1

kayman-mk marked this pull request as ready for review April 20, 2023 10:15

kayman-mk requested a review from npalm as a code owner April 20, 2023 10:15

kayman-mk mentioned this pull request Apr 20, 2023

refactor!: group variables for better overview #810

Merged

kayman-mk force-pushed the refactor-variables branch from 26a6d19 to dc5a758 Compare April 27, 2023 06:54

merge refactor-variables

278684a

kayman-mk closed this Apr 27, 2023

kayman-mk deleted the kayma/split-variables branch June 15, 2023 19:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: rename variables and prefix with `runner_manager`, `runner`, `runner_worker` and global scope #757

refactor!: rename variables and prefix with `runner_manager`, `runner`, `runner_worker` and global scope #757

kayman-mk commented Mar 20, 2023 •

edited

Loading

github-actions bot commented Mar 20, 2023

github-actions bot commented Mar 20, 2023 •

edited

Loading

kayman-mk commented Mar 20, 2023

tmeijn commented Mar 21, 2023

kayman-mk commented Mar 21, 2023

tmeijn commented Apr 5, 2023

kayman-mk commented Apr 20, 2023

kayman-mk commented Apr 20, 2023

kayman-mk commented Apr 27, 2023

refactor!: rename variables and prefix with runner_manager, runner, runner_worker and global scope #757

refactor!: rename variables and prefix with runner_manager, runner, runner_worker and global scope #757

Conversation

kayman-mk commented Mar 20, 2023 • edited Loading

Description

Migrations required

Verification

github-actions bot commented Mar 20, 2023

github-actions bot commented Mar 20, 2023 • edited Loading

🦙 MegaLinter status: ❌ ERROR

kayman-mk commented Mar 20, 2023

tmeijn commented Mar 21, 2023

kayman-mk commented Mar 21, 2023

tmeijn commented Apr 5, 2023

kayman-mk commented Apr 20, 2023

kayman-mk commented Apr 20, 2023

kayman-mk commented Apr 27, 2023

refactor!: rename variables and prefix with `runner_manager`, `runner`, `runner_worker` and global scope #757

refactor!: rename variables and prefix with `runner_manager`, `runner`, `runner_worker` and global scope #757

kayman-mk commented Mar 20, 2023 •

edited

Loading

github-actions bot commented Mar 20, 2023 •

edited

Loading