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.

Sign up for GitHub

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

Jump to bottom

fix: Rebuild examples #1625

Merged
merged 11 commits into from
Oct 12, 2021
Merged

fix: Rebuild examples #1625

merged 11 commits into from
Oct 12, 2021

Conversation

daroga0002
Copy link
Contributor

@daroga0002 daroga0002 commented Oct 6, 2021 •
edited by antonbabenko
Loading

PR o'clock

Description

resolves #1622

This is major improvement in our examples. Some examples were broken or were not running correctly, after those fixes:

  • all examples are working as should (I tested each of them)
  • examples have generic README.md which can be updated more over time
  • in IRSA example I automated helm autoscaler deployment in terraform (it was manual step which was error prone)
  • fixed bottlerocket example where nodes were not registering to cluster (missing aws-auth configmap management)
  • each example is now independent and has random suffix (so can be used multiple times on same AWS account)
  • each example creates vpc and other basic resources
  • core-dns is scheduled correctly in fargate example
  • fixed variables description highlighted in fix description of variable "map_users" #1623
  • fixed some markdown linting error in main README.md
  • bump some versions.tf in examples to align with overall module
  • removed _bootstrap example as it was created dependencies and in general didn't allow to run multiple examples in parallel

Checklist

@daroga0002 daroga0002 changed the title Rebuild examples fix: rebuild examples Oct 6, 2021
@antonbabenko
Copy link
Member

Good improvement overall!

The changes are rather big, and not all of them are in sync with the approach we have in other modules in this organization (e.g. different structures, file names, etc). I will try to find time to review it during this week.

@daroga0002 daroga0002 force-pushed the rebuild-examples branch 3 times, most recently from 1d40b81 to cc8c7d7 Compare October 8, 2021 07:05
antonbabenko
antonbabenko requested changes Oct 8, 2021
View reviewed changes
Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Hi @daroga0002 ! I reviewed it and most of the comments are applicable to all examples.

examples/bottlerocket/generic.tf Outdated Show resolved Hide resolved
examples/bottlerocket/generic.tf Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
examples/bottlerocket/README.md Outdated Show resolved Hide resolved
examples/bottlerocket/generic.tf Outdated Show resolved Hide resolved
examples/irsa/irsa.tf Outdated Show resolved Hide resolved
examples/irsa/versions.tf Outdated Show resolved Hide resolved
examples/launch_templates/README.md Outdated Show resolved Hide resolved
examples/launch_templates_with_managed_node_groups/README.md Outdated Show resolved Hide resolved
examples/managed_node_groups/README.md Outdated Show resolved Hide resolved
@daroga0002
Copy link
Contributor Author

@antonbabenko I reviewed comments, some I will fix, some I answered here for discussion as I truly believe those changes should be done to make it more flexible for example for testing purposes

@daroga0002
Copy link
Contributor Author

first round of fixes from review, there will be also second commit

@daroga0002 daroga0002 force-pushed the rebuild-examples branch 2 times, most recently from d0634c4 to ed4b41d Compare October 12, 2021 08:46
antonbabenko
antonbabenko reviewed Oct 12, 2021
View reviewed changes
Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Minor comment about ordering of resources in examples. LGTM.

examples/bottlerocket/main.tf
region = "eu-west-1"
}

################################################################################
Copy link
Member

Choose a reason for hiding this comment

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

Please move all supporting resources and data sources after the most important piece in the example - module "eks".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@daroga0002
Copy link
Contributor Author

after this I think we can publish a release as this will be one of biggest releases for very long

@antonbabenko antonbabenko changed the title fix: rebuild examples fix: Rebuild examples Oct 12, 2021
@antonbabenko antonbabenko merged commit 99d2899 into terraform-aws-modules:master Oct 12, 2021
@antonbabenko
Copy link
Member

Thanks @daroga0002 !

v17.21.0 has been just released.

@daroga0002 daroga0002 deleted the rebuild-examples branch October 12, 2021 13:23
lisfo4ka pushed a commit to lisfo4ka/terraform-aws-eks that referenced this pull request Oct 12, 2021
@daroga0002
fix: Rebuild examples (terraform-aws-modules#1625)
a44aacd
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@antonbabenko antonbabenko antonbabenko requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Multiple examples are not working
2 participants
@daroga0002 @antonbabenko