Skip to content

Conversation

kayman-mk
Copy link
Collaborator

@kayman-mk kayman-mk commented Jun 19, 2022

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.

module "gitlab_ci_runner" {
  ...
  runners_docker_options {
    # set whatever is necessary
  }

Verification

  • Use current configuration and ensure that the config.toml remains unchanged
  • Set all new block variables and ensure that the config.toml is valid (use `gitlab-runner verify)
  • Check that the default settings with Terraform 1.3 work as expected
  • 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.

@kayman-mk
Copy link
Collaborator Author

Make sure to write the toml file with correct data types, e.g. boolean and numbers without ""

@kayman-mk
Copy link
Collaborator Author

kayman-mk commented Jun 22, 2022

@npalm Not a good idea?
There are so many variables and many more to come as the issues here indicate (#467, #491, #513). Thought it would be better to place them in a map for a cleaner module interface.

@kayman-mk
Copy link
Collaborator Author

kayman-mk commented Jun 23, 2022

A block would be an alternative solution.

runners_docker_options {
  shm_size =0
}

@JulianCBC
Copy link
Contributor

Could we get rid of the runners_enable_docker_options variable and maintain backwards compatibility by making the default just use the old variables?

We'd have to note on the old variables that if runners_docker_options is set, then they're ignored, but that'd simplify a lot of the code here.

@kayman-mk
Copy link
Collaborator Author

Sure. But setting a default for the new block is problematic. I have to know whether the new block is used or the old variables.

We could set a default null and then decide: if var.runners_docker_options == null then use old variables else use new block variables.

As soon as the old variables are removed we could set the default for the block.

@JulianCBC
Copy link
Contributor

Other than enabling the experiment which I can't see actually being used right now, this looks good to me!

I can't help but wonder if you could use the template for both the original and new variable sets, and do all the formatting of the volumes string in the template, but those are pretty minor quibbles and the code I'm complaining about is deprecated.

@kayman-mk
Copy link
Collaborator Author

Yeah, the experiment is not my favorite too. But it made it into Terraform 1.3.0 (released soon). Default values will also be possible.

@kayman-mk
Copy link
Collaborator Author

Good point though. Reworked that section. Looks much better now.

@kayman-mk
Copy link
Collaborator Author

Looks better but doesn't work. I always access null variables. Reverted.

@npalm
Copy link
Collaborator

npalm commented Jul 16, 2022

Would be great when we can move to Terrfaform 1.3. The optiional in object will made the code a lot cleaner on my places. Just checked, there are two alphaversions out. So it is still in an early state but at least we see the progress towards 1.3

@kayman-mk
Copy link
Collaborator Author

Still waiting for the Terraform 1.3 release. No idea when this will happen.

@kayman-mk kayman-mk force-pushed the kayma/network-mode branch from 7befe11 to 9dd1a97 Compare March 16, 2023 19:49
@kayman-mk kayman-mk marked this pull request as draft March 16, 2023 19:49
@kayman-mk kayman-mk marked this pull request as ready for review March 16, 2023 20:17
@demisx
Copy link

demisx commented Mar 19, 2023

Can this be finalized and merged soon? We need to change wait_for_services_timeout param in runners.docker section and it appears there is no way to do so until this PR is merged. Thank you.

@tmeijn
Copy link
Contributor

tmeijn commented Mar 20, 2023

FYI: services_security_opt made it into GitLab Runner 15.10 today... https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/3760. It's not documented yet though...

@kayman-mk kayman-mk force-pushed the refactor-variables branch from 3fdd3c5 to c78907a Compare March 23, 2023 14:40
@kayman-mk kayman-mk force-pushed the refactor-variables branch 2 times, most recently from babc50a to 3662eeb Compare April 20, 2023 07:22
@kayman-mk
Copy link
Collaborator Author

@npalm Conflicts solved and merged into the upcoming major release branch.

@kayman-mk kayman-mk merged commit 26a6d19 into cattle-ops:refactor-variables Apr 20, 2023
kayman-mk added a commit that referenced this pull request Apr 27, 2023
## 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 added a commit that referenced this pull request May 3, 2023
## 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 added a commit that referenced this pull request May 11, 2023
## 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 added a commit that referenced this pull request Jun 1, 2023
## 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 added a commit that referenced this pull request Jun 15, 2023
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.

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
  }
```

- [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 added a commit that referenced this pull request Jul 6, 2023
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.

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
  }
```

- [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 added a commit that referenced this pull request Jul 6, 2023
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.

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
  }
```

- [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 added a commit that referenced this pull request Sep 7, 2023
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.

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
  }
```

- [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 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.

Add network_mode to docker.runner config
5 participants