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

1.25 #341

Merged
merged 18 commits into from
Jun 21, 2023
Merged

1.25 #341

merged 18 commits into from
Jun 21, 2023

Conversation

errm
Copy link
Member

@errm errm commented Jun 6, 2023

  • Upgrades to k8s 1.25
  • Moves to a much more minimal setup
    • ASG node groups are no longer supported
    • Karpenter must be used to provision nodes
      • Karpenter is run via fargate
    • All addons / cluster configuration is removed from this module.
      • Recommended to manage via flux or other gitops

📝 TODO

  • Upgrade procedure / instructions!

@errm errm requested a review from a team as a code owner June 6, 2023 14:34
@errm errm requested review from pray and jportasa June 6, 2023 15:49
fstype = "csi.storage.k8s.io/fstype: ${var.pv_fstype}",
}
)
resource "kubernetes_config_map_v1_data" "aws_auth" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this block is very similar to resource "kubernetes_config_map" "aws_auth" why we have this twice?

Copy link
Member Author

Choose a reason for hiding this comment

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

The kubernetes_config_map creates the configmap, whereas this resource manages the content of the data field.

Managing this this way means we get changes to the data in the terraform plan.

I copied the idea for this from this module https://github.com/terraform-aws-modules/terraform-aws-eks/blob/master/main.tf#L536-L569

]
},
{
rolearn = aws_iam_role.karpenter_node.arn
Copy link
Contributor

Choose a reason for hiding this comment

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

In web-platform-X we use instanceProfile: EKSNode here we will have to change it in the provisioner?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is the intention, but we can add the old profile name to the map during transition.

@@ -0,0 +1,74 @@
resource "aws_sqs_queue" "karpenter_interruption" {
Copy link
Contributor

@jportasa jportasa Jun 8, 2023

Choose a reason for hiding this comment

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

maybe more clear to rename this file to karpenter_spot_node_interruption_queue.tf?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it makes it clearer, this queue will have a number of different events - not only spot interruptions.

@@ -0,0 +1,74 @@
resource "aws_sqs_queue" "karpenter_interruption" {
name = "Karpenter-${var.name}"
Copy link
Contributor

@jportasa jportasa Jun 8, 2023

Choose a reason for hiding this comment

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

maybe more clear karpenter-spot-node-interruption-${var.name}?

variables.tf Show resolved Hide resolved
required_providers {
aws = {
source = "hashicorp/aws"
version = ">= 4.47.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

5.1.0 maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

>= 4.47.0 covers this, since we expect other projects to depend on this module we should have loose constraints (within reason).


kubernetes = {
source = "hashicorp/kubernetes"
version = ">= 2.10"
Copy link
Contributor

Choose a reason for hiding this comment

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

2.21.1 is the latest

Copy link
Member Author

Choose a reason for hiding this comment

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

Same

karpenter_fargate.tf Outdated Show resolved Hide resolved
The design here is that new clusters all have a new IAM role per
cluster. But to avoid recreating legacy clusters on upgrade, we
need a way to use an externally managed IAM role.
@jportasa jportasa merged commit f5918ba into main Jun 21, 2023
3 checks passed
@errm errm deleted the errm/1-25-minimal branch October 6, 2023 14:06
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.

2 participants