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

Add Ondat #255

Merged
merged 8 commits into from
Apr 17, 2022
Merged

Add Ondat #255

merged 8 commits into from
Apr 17, 2022

Conversation

cvlc
Copy link
Contributor

@cvlc cvlc commented Feb 14, 2022

What does this PR do?

This PR provides a Terraform module to install a fully-fledged Ondat cluster using EKS, Helm and using a few Ondat OSS Terraform modules for etcd and storage.

Motivation

This contribution is aimed to provide an easy path for users to deploy Ondat within AWS with EKS.

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I have added a new example under examples to support my PR
  • Yes, I have created another PR for add-ons under add-ons repo (if applicable)
  • Yes, I have updated the docs for this feature
  • Yes, I ran pre-commit run -a with this PR

Note: Not all the PRs required examples and docs except a new pattern or add-on added.

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

  • An Ubuntu for EKS AMI is required for Ondat worker nodes as Amazon Linux and Bottlerocket currently lack the required kernel module target_core_user. A feature request has been raised to resolve this.
  • An external etcd cluster is currently required for Ondat, this is deployed using the MIT-licensed Terraform module etcd3-terraform with the binary etcd3-bootstrap used to attach EBS at runtime to persist storage through the EKS node lifecycle.

@vara-bonthu
Copy link
Contributor

Nice work on this Calum @cvlc 🙌 Thanks for adding this Add-on. 👍🏼
We are currently working on a Partner contribution docs that will be published to this repo soon. We will get back to you soon on this.

@kcoleman731
Copy link
Contributor

@cvlc thanks again for this PR!

We merged the extensibility guide for partners last week. https://github.com/aws-samples/aws-eks-accelerator-for-terraform/blob/main/docs/extensibility.md. As you will see in the doc, the recommendation is for partners to maintain their add-ons in a repo that they own. This allows you to evolve your add-on independent of the core repo.

Please review the guide and let us know if you have any questions.

@askulkarni2
Copy link
Contributor

Hello @cvlc. Did you have a chance to look at this comment? We also recently merged Tetrate Istio which you can look at a reference example for a partner addon. Please let us know if you need any additional guidance.

@cvlc
Copy link
Contributor Author

cvlc commented Mar 31, 2022

Thanks all, I've made updates according to the documentation and examples provided!

Comment on lines 176 to 214
variable "ondat_etcd_endpoints" {
type = list(string)
default = []
description = "List of etcd endpoints for Ondat"
}

variable "ondat_etcd_ca" {
type = string
default = null
description = "CA content for Ondat etcd"
}

variable "ondat_etcd_cert" {
type = string
default = null
description = "Certificate content for Ondat etcd"
}

variable "ondat_etcd_key" {
type = string
default = null
sensitive = true
description = "Private key content for Ondat etcd"
}

variable "ondat_admin_username" {
type = string
default = "storageos"
description = "Username for Ondat admin user"
}

variable "ondat_admin_password" {
type = string
default = "storageos"
sensitive = true
description = "Password for Ondat admin user"
}


Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice fit these additional variables into one complex type variable. I am concerned that number of variables growing heavily for k8s add-ons module. Its nice improvement if you want to look into it

Copy link
Contributor

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

@cvlc LGTM 👍🏼
One change is required for version tagging . The other change is nice to have

Also uses Terraform registry over GitHub
@cvlc
Copy link
Contributor Author

cvlc commented Apr 4, 2022

Thanks - I've pinned the version and switched to the Terraform registry as module source. I did try with a combined object variable but I found it easier to debug with multiple - it can get a bit confusing when you're working with a massive object, especially if you mark it sensitive in the state!

@cvlc cvlc requested a review from vara-bonthu April 4, 2022 10:48
@cvlc
Copy link
Contributor Author

cvlc commented Apr 6, 2022

Hi all,
I came across an issue with passing a resource attribute directly through to a list that's iterated on via for_each in v0.0.1, after fixing it I've released v0.0.2.

Copy link
Contributor

@askulkarni2 askulkarni2 left a comment

Choose a reason for hiding this comment

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

@cvlc thanks so much for the PR and the updates. One requested change from my side. Here https://github.com/ondat/terraform-eksblueprints-ondat-addon/blob/d1f4dd18315b93ceca554bb6f209f4b6d1c5d64a/main.tf#L12, please pin source to version ref=v4.0.1 (assuming you have tested with that).

@cvlc
Copy link
Contributor Author

cvlc commented Apr 8, 2022

@cvlc cvlc requested a review from askulkarni2 April 8, 2022 13:01
@cvlc
Copy link
Contributor Author

cvlc commented Apr 12, 2022

Hi all,

Now the repo has been renamed I've released a new version and updated all references. The module now requires a manual edit to the example to this ref for the kubernetes_addon module to work, in preparation for merge to main of this repo.

Copy link
Contributor

@askulkarni2 askulkarni2 left a comment

Choose a reason for hiding this comment

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

LGTM!

@askulkarni2
Copy link
Contributor

Hi @cvlc, thanks for the updates. The code changes look good. However, I am seeing the following when I test the blueprint example from your repo. Can you please check at your end?

▶ k get pods -n storageos
NAME                                   READY   STATUS             RESTARTS   AGE
ondat-ondat-operator-c8b7d4fd5-nckjz   2/2     Running            0          7m26s
storageos-node-2b8jt                   2/3     CrashLoopBackOff   5          6m56s
storageos-node-ct6s9                   2/3     CrashLoopBackOff   5          6m56s
storageos-node-sfkbj                   2/3     CrashLoopBackOff   5          6m56s
storageos-scheduler-5ddc468b54-dwlgq   1/1     Running            0          7m6s

Upon checking the logs for one of the pods, I see the following error..

{"endpoints":["https://etcd.us-east-2.i.a.aws001-preprod-dev-etcd.int:2379"],"error":"context deadline exceeded","level":"error","msg":"unable to connect to etcd","store":"etcd","time":"2022-04-13T21:35:27.75302845Z"}
{"error":"failed to instantiate ETCD: context deadline exceeded","level":"error","msg":"failed to initialise store client","time":"2022-04-13T21:35:27.762018823Z"}
{"level":"info","msg":"shutting down","time":"2022-04-13T21:35:27.762108558Z"}

@cvlc
Copy link
Contributor Author

cvlc commented Apr 14, 2022

Thanks - I've figured out the issue and will update when it's resolved!

@cvlc
Copy link
Contributor Author

cvlc commented Apr 14, 2022

Hi all,

The problems are now resolved! Please do try again after a terraform get -update. I have updated etcd and Ondat, merged in @askulkarni2's suggestion for adding a Helm provider block and have made some changes to the etcd Terraform logic and EBS bootstrap component to work with the latest AWS and EKS Ubuntu images.

Copy link
Contributor

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

Thanks @cvlc 👍🏼 LGTM

@askulkarni2 askulkarni2 merged commit f1d5d1f into aws-ia:main Apr 17, 2022
alidonmez pushed a commit to alidonmez/terraform-aws-eks-blueprints-1 that referenced this pull request Mar 25, 2023
…n/test/util/yaml-2.2.1

Bump yaml from 2.1.3 to 2.2.1 in /test/util
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.

None yet

5 participants