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: migrates example on eks-cluster-aws-4.x #173

Merged
merged 3 commits into from
May 8, 2024
Merged

Conversation

gberenice
Copy link
Contributor

what

  • Upgrade the example to use eks-cluster v4.x.x, where any dependencies on the Kubernetes provider were removed.

why

  • This eliminates the Terraform test error caused by the kubernetes provider issue. As a consequence, this unlocks merging the PRs. Example of the error:
Error: Received unexpected error:
FatalError{Underlying: error while running command: exit status 1; ╷
     │ Error: Value Conversion Error
     │ 
     │ with module.eks_cluster.provider["registry.terraform.io/hashicorp/kubernetes"],
     │ on .terraform/modules/eks_cluster/auth.tf line 96, in provider "kubernetes":
     │ 96: provider "kubernetes" {

references

@gberenice gberenice requested review from a team as code owners April 29, 2024 10:14
@gberenice gberenice requested review from kevcube and jamengual April 29, 2024 10:14
@gberenice
Copy link
Contributor Author

gberenice commented Apr 29, 2024

These changes were released in v3.0.0.

@mergify mergify bot added the triage Needs triage label Apr 29, 2024
@gberenice
Copy link
Contributor Author

/terratest

@joe-niland
Copy link
Member

Seems OK, as it's based on the referenced PR

@mergify mergify bot removed the triage Needs triage label May 8, 2024
@gberenice gberenice enabled auto-merge (squash) May 8, 2024 08:01
@gberenice
Copy link
Contributor Author

/terratest

@gberenice gberenice merged commit fa3d05a into main May 8, 2024
10 checks passed
@gberenice gberenice deleted the eks-cluster-aws-4.x branch May 8, 2024 08:27
@Nuru
Copy link
Contributor

Nuru commented May 10, 2024

@joe-niland @gberenice Thank you for this PR.

For future reference:

  1. Any PRs that do not change the code of the Terraform module itself should be labeled no-release to avoid triggering the release of a new module version upon merging the PR. Without this label, a new release may be created and people and systems feel the need to update to the new release, but the new release is exactly the same code, and it is just a waste of time and resources to upgrade. There are filters in place to try to catch PRs like this one that do not change any code and therefore do not trigger a release, and this PR was caught by the filter, but still, it is best practice to label the PR no-release to be on the safe side.

    Side note: you can also use no-release when you plan to merge several PRs in one day, so that you can batch the changes and reduce the upgrade noise.

  2. PRs that do change the code, but only make small changes, should get patch or bugfix labels. I mention it because although this PR should have had the no-release label, failing that, it should have had the patch label.

  3. Also, please note that any change to the Kubernetes version in the example requires a corresponding change to the Kubernetes Go packages in test/src/go.mod when such packages are present:

    k8s.io/api v0.25.12
    k8s.io/client-go v0.25.12

    And then, of course, changing the go.mod file requires updating the go.sum file with go mod tidy.

See #177 for example.

k8s.io/api v0.29.4
k8s.io/client-go v0.29.4

Specifically, the minor version numbers must match. Here K8s version 1.25.x used packages versions v0.25.x and the update to K8s v1.29.x requires updating the packages to v0.29.x. The patch levels do not need to match.

@gberenice
Copy link
Contributor Author

Thanks for the explanation @Nuru. I feel like these important details should be described in the community docs, but I can't find this info there. Am I missing something?

@Nuru
Copy link
Contributor

Nuru commented May 10, 2024

Thanks for the explanation @Nuru. I feel like these important details should be described in the community docs, but I can't find this info there. Am I missing something?

@gberenice You are right, they should be, but are not, documented there. I made a note to update the Code Review Guidelines.

@joe-niland
Copy link
Member

@Nuru thanks for the notes. They are very helpful.

Agreed that it will be beneficial to document them.

It might also be helpful to update CONTRIBUTING because it covers labelling and updating of the examples.

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