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

feat: do not add a the Name tag for docker+machine >= 0.16.2 #522

Merged
merged 6 commits into from
Aug 15, 2022

Conversation

kayman-mk
Copy link
Collaborator

@kayman-mk kayman-mk commented Aug 1, 2022

Description

This PR checks the version of the docker machine and adds the Name tag if it is not done by the driver itself. It also removes the second Name tag from the amazonec2-tags in the config.toml. In consequence it is no longer possible to change the Name of the Runners as the Name tag is now added by the docker-machine driver (for versions >= 0.16.2).

docker_machine_version >= 0.16.2 automatically adds the Name tag to the EC2 instances. Adding another Name tag here causes the EC2 creation process to fail.

To be able to configure the name, var.overrides["name_docker_machine_runners"] is added as prefix to the name of the docker machine which is used as name for the EC2 instance by the driver.

The main logic was taken from hashicorp/terraform#22688 (comment)

Closes #521

Migrations required

No.

Verification

  • Verify the version logic manually with a local.tf only. Returns true if version >= 0.16.2
  • Check the config.toml for versions >= 0.16.2 with overrides["name_docker_machine_runners"]. No Name tag present.
  • Check the config.toml for versions >= 0.16.2 without overrides["name_docker_machine_runners"]. No Name tag present.
  • Check the config.toml for versions < 0.16.2 with overrides["name_docker_machine_runners"]. No Name tag equals the name set in the override.
  • Check the config.toml for versions < 0.16.2 without overrides["name_docker_machine_runners"]. Name tag shows var.environment + "-docker-machine"
  • Replace the current module with this version and check if tags are changed (using version < 0.16.2). There shouldn't be any change in the resource tags.

@kayman-mk kayman-mk marked this pull request as draft August 1, 2022 13:10
@kayman-mk kayman-mk marked this pull request as ready for review August 6, 2022 07:20
@npalm npalm self-requested a review August 8, 2022 20:29
@kayman-mk kayman-mk changed the title feat: do not a the Name tag for docker+machine >= 0.16.2 feat: do not add a the Name tag for docker+machine >= 0.16.2 Aug 9, 2022
@kayman-mk
Copy link
Collaborator Author

kayman-mk commented Aug 9, 2022

Also see #521 for long-wan-ep's test.

@kayman-mk
Copy link
Collaborator Author

Still discussing the MachineName option in #521

@kayman-mk
Copy link
Collaborator Author

kayman-mk commented Aug 13, 2022

Using the var.overrides["name_docker_machine_runners"] as prefix for the docker machine name. There is no change if docker machine < 0.16.2 is used (these versions use the Name tag provided), but since 0.16.2 the prefix will show up in the name of the EC2 instance (name of docker machine = name of EC2 instance).

@long-wan-ep
Copy link
Contributor

long-wan-ep commented Aug 15, 2022

Tested out the prefix for the machine name, it looks good to me. One thing to note is that gitlab-runners will add runner-<SHORT_TOKEN> as a prefix to the machine name so the name of the instance will end up being runner-<SHORT_TOKEN>-<NAME_OVERRIDE>-<MACHINE_IDENTIFIER>, but I think it's ok.

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.

LGTM, tested with default example

@npalm npalm merged commit 7e6d9be into cattle-ops:develop Aug 15, 2022
semantic-releaser bot pushed a commit that referenced this pull request Aug 15, 2022
## [5.2.0](5.1.0...5.2.0) (2022-08-15)

### Features

* do not add a the Name tag for docker+machine >= 0.16.2 ([#522](#522)) ([7e6d9be](7e6d9be))

### Bug Fixes

* always add the cache policy ([#528](#528)) ([ccaf55d](ccaf55d))
@semantic-releaser
Copy link
Contributor

🎉 This PR is included in version 5.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Duplicate tag key 'Name' error with newer docker-machine version
3 participants