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

refactor!: group variables for better overview #810

Merged
merged 82 commits into from
May 22, 2023

Conversation

kayman-mk
Copy link
Collaborator

@kayman-mk kayman-mk commented Apr 20, 2023

Description

Groups variables into objects to

  • reduce the number of variables (currently 118)
  • to gain a better overview of all configuration settings

Creates new groups of variables:

  • 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, ...

Migrations required

Yes and a script is provided to do that. It covers 98% of all migrations (see migrations/migrate-to-7-0-0.sh)

Verification

Please mention the examples you have verified.

tmeijn and others added 30 commits March 16, 2023 08:57
## 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 <[email protected]>
## 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 <[email protected]>
## 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 <[email protected]>
## 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 <[email protected]>
…autoscaling options (#711)

## Description

Switches from hardcoded options to free-from scaling configuration. This
reduces the module complexity by allowing to get rid of a number of
variables while giving more control to the user to define their options
without us having to build support into it for.

Adds `idle_scale_factor` and `idle_count_min` Docker Machine options.
See
[documentation](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runnersmachine-section").

## Migrations required

YES - users will have to change the input name from
`runners_machine_autoscaling` to `runners_machine_autoscaling_options`.
No other changes should be needed, we just support _more_ options. A
migration script is available.

## Verification

No input given:

(end of rendered `config.toml`)


![image](https://user-images.githubusercontent.com/17970041/225890782-02fe4adc-4c6a-4237-9752-a64349464113.png)

Input:

```hcl

runners_machine_autoscaling_options = [
    {
      periods           = ["* * 9-17 * * mon-fri *", "* * 9-17 * * mon-fri *"]
      idle_count        = 50
      idle_count_min    = 10
      idle_time         = 3600
      timezone          = "UTC"
      idle_scale_factor = 1.5
    },
    {
      periods    = ["* * 9-17 * * mon-fri *", "* * 9-17 * * mon-fri *"]
      idle_count = 50
      idle_time  = 3600
      timezone   = "Europe/Amsterdam"
    }
  ]
```

Rendered `config.toml`:


![image](https://user-images.githubusercontent.com/17970041/225891085-add03ee8-3943-4c56-96a4-d1a8c252deb0.png)

Apply results:


![image](https://user-images.githubusercontent.com/17970041/225893020-a9850486-4aa6-4eb0-b996-558ec7bccfea.png)


Closes #556

---------

Co-authored-by: Matthias Kay <[email protected]>
## 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 <[email protected]>
kayman-mk and others added 3 commits May 3, 2023 08:48
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 <[email protected]>

# Conflicts:
#	main.tf
…autoscaling options (#711)

## Description

Switches from hardcoded options to free-from scaling configuration. This
reduces the module complexity by allowing to get rid of a number of
variables while giving more control to the user to define their options
without us having to build support into it for.

Adds `idle_scale_factor` and `idle_count_min` Docker Machine options.
See
[documentation](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runnersmachine-section").

## Migrations required

YES - users will have to change the input name from
`runners_machine_autoscaling` to `runners_machine_autoscaling_options`.
No other changes should be needed, we just support _more_ options. A
migration script is available.

## Verification

No input given:

(end of rendered `config.toml`)


![image](https://user-images.githubusercontent.com/17970041/225890782-02fe4adc-4c6a-4237-9752-a64349464113.png)

Input:

```hcl

runners_machine_autoscaling_options = [
    {
      periods           = ["* * 9-17 * * mon-fri *", "* * 9-17 * * mon-fri *"]
      idle_count        = 50
      idle_count_min    = 10
      idle_time         = 3600
      timezone          = "UTC"
      idle_scale_factor = 1.5
    },
    {
      periods    = ["* * 9-17 * * mon-fri *", "* * 9-17 * * mon-fri *"]
      idle_count = 50
      idle_time  = 3600
      timezone   = "Europe/Amsterdam"
    }
  ]
```

Rendered `config.toml`:


![image](https://user-images.githubusercontent.com/17970041/225891085-add03ee8-3943-4c56-96a4-d1a8c252deb0.png)

Apply results:


![image](https://user-images.githubusercontent.com/17970041/225893020-a9850486-4aa6-4eb0-b996-558ec7bccfea.png)


Closes #556

---------

Co-authored-by: Matthias Kay <[email protected]>
## 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 <[email protected]>
@kayman-mk kayman-mk force-pushed the refactor-variables branch from dc5a758 to 7e05787 Compare May 3, 2023 06:50
@kayman-mk
Copy link
Collaborator Author

@npalm Merged changes of 6.4.0 release. Please have a look. Testing to be done in target branch refactor-variables.

Copy link
Collaborator

@npalm npalm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks in general good. Will run a check also locally.

@npalm
Copy link
Collaborator

npalm commented May 7, 2023

Scopting with refactor will not create a new release, and also will not be mentioned int he release note as far I know. Maybe better to just scope it as feat.

@kayman-mk
Copy link
Collaborator Author

kayman-mk commented May 8, 2023

The ! should be the solution as it creates a major release. Let me quickly check this.

Edit: Works as expected: https://github.com/Hapag-Lloyd/test/pull/31

@kayman-mk
Copy link
Collaborator Author

@npalm The only problem I see, is, how to manually update the release docs. Tried some changes here and there, but Release-Please overwrites them. Any idea how to change the docs manually?

I would like to have a reference to the migration script at a prominent place.

@kayman-mk kayman-mk force-pushed the refactor-variables branch from 7e05787 to 824ce7a Compare May 11, 2023 08:15
@kayman-mk
Copy link
Collaborator Author

@npalm Rebasing is a pain here. Can we merge to target branch kayma/group-variables to start testing the v7 release?

@kayman-mk
Copy link
Collaborator Author

Merged into the pre-release branch for testing.

@kayman-mk kayman-mk merged commit bd1cbf6 into refactor-variables May 22, 2023
@kayman-mk kayman-mk deleted the kayma/group-variables branch May 22, 2023 18:42
kayman-mk added a commit that referenced this pull request Jun 1, 2023
## Description

Groups variables into objects to
- reduce the number of variables (currently 118)
- to gain a better overview of all configuration settings

Creates new groups of variables:
- `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, ...

## Migrations required

Yes and a script is provided to do that. It covers 98% of all migrations
(see migrations/migrate-to-7-0-0.sh)

## Verification

Please mention the examples you have verified.

---------

Co-authored-by: Tyrone Meijn <[email protected]>
kayman-mk added a commit that referenced this pull request Jun 15, 2023
Groups variables into objects to
- reduce the number of variables (currently 118)
- to gain a better overview of all configuration settings

Creates new groups of variables:
- `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, ...

Yes and a script is provided to do that. It covers 98% of all migrations
(see migrations/migrate-to-7-0-0.sh)

Please mention the examples you have verified.

---------

Co-authored-by: Tyrone Meijn <[email protected]>
kayman-mk added a commit that referenced this pull request Jul 6, 2023
Groups variables into objects to
- reduce the number of variables (currently 118)
- to gain a better overview of all configuration settings

Creates new groups of variables:
- `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, ...

Yes and a script is provided to do that. It covers 98% of all migrations
(see migrations/migrate-to-7-0-0.sh)

Please mention the examples you have verified.

---------

Co-authored-by: Tyrone Meijn <[email protected]>
kayman-mk added a commit that referenced this pull request Jul 6, 2023
Groups variables into objects to
- reduce the number of variables (currently 118)
- to gain a better overview of all configuration settings

Creates new groups of variables:
- `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, ...

Yes and a script is provided to do that. It covers 98% of all migrations
(see migrations/migrate-to-7-0-0.sh)

Please mention the examples you have verified.

---------

Co-authored-by: Tyrone Meijn <[email protected]>
kayman-mk added a commit that referenced this pull request Sep 7, 2023
Groups variables into objects to
- reduce the number of variables (currently 118)
- to gain a better overview of all configuration settings

Creates new groups of variables:
- `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, ...

Yes and a script is provided to do that. It covers 98% of all migrations
(see migrations/migrate-to-7-0-0.sh)

Please mention the examples you have verified.

---------

Co-authored-by: Tyrone Meijn <[email protected]>
kayman-mk pushed a commit that referenced this pull request Sep 10, 2023
🤖 I have created a release *beep* *boop*
---


##
[7.0.0](6.5.2...7.0.0)
(2023-09-09)


### ⚠ BREAKING CHANGES

* group variables for better overview
([#810](#810))
* allow to set all docker options for the Executor
([#511](#511))
* add idle_count_min` and `idle_scale_factor` to Docker Machine
autoscaling options
([#711](#711))
* remove deprecated variables
([#738](#738))
* remove deprecated pull policy variable
([#710](#710))

### Features

* add idle_count_min` and `idle_scale_factor` to Docker Machine
autoscaling options
([#711](#711))
([1538d48](1538d48))
* allow to set all docker options for the Executor
([#511](#511))
([461561e](461561e))


### Bug Fixes

* add missing defaults
([#905](#905))
([eb44182](eb44182))
* correct the bugs of major version 7 (pre-release)
([#860](#860))
([f236b58](f236b58))
* remove deprecated pull policy variable
([#710](#710))
([8736ec7](8736ec7))


### Miscellaneous Chores

* remove deprecated variables
([#738](#738))
([676ed6a](676ed6a))


### Code Refactoring

* group variables for better overview
([#810](#810))
([c8a3b89](c8a3b89))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Signed-off-by: Niek Palm <[email protected]>
Co-authored-by: cattle-ops-releaser-2[bot] <134548870+cattle-ops-releaser-2[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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.

3 participants