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: Add terraform module #200

Merged
merged 4 commits into from
Sep 24, 2024
Merged

feat: Add terraform module #200

merged 4 commits into from
Sep 24, 2024

Conversation

orfeas-k
Copy link
Contributor

@orfeas-k orfeas-k commented Sep 18, 2024

Create a terraform/ directory that hosts the Terraform module for this charm. It follows the structure proposed in this spec and it is follows what was done in canonical/argo-operators#198.

To test the module:

  • Clone the repository and switch this PR's branch.
  • First run tox -e tflint to ensure that linting is correct
  • Run terraform apply -var "channel=latest/edge" -var "model_name=kubeflow" --auto-approve and wait until the charm is Active and Idle.

Ref #197

Copy link
Contributor

@DnPlas DnPlas left a comment

Choose a reason for hiding this comment

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

Thanks @orfeas-k, left a comment. Similar to your PR for notebook operators, I think it would be beneficial if we set the default channel to latest/edge in the re-usable wf. Let me know your decision and if you change this PR to reflect that change, as that was the only thing I noticed.

Copy link
Contributor

@DnPlas DnPlas left a comment

Choose a reason for hiding this comment

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

Thanks @orfeas-k, few comments that may apply to other PRs as well.

terraform/README.md Outdated Show resolved Hide resolved
.github/workflows/integrate.yaml Outdated Show resolved Hide resolved
@orfeas-k
Copy link
Contributor Author

DnPlas
DnPlas previously requested changes Sep 24, 2024
Copy link
Contributor

@DnPlas DnPlas left a comment

Choose a reason for hiding this comment

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

@orfeas-k just a couple of changes, other than that lgtm!

.github/workflows/integrate.yaml Outdated Show resolved Hide resolved
terraform/README.md Outdated Show resolved Hide resolved
@orfeas-k
Copy link
Contributor Author

@DnPlas thank you for your contribution to this effort, updated!

Copy link
Contributor

@mvlassis mvlassis left a comment

Choose a reason for hiding this comment

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

Nice work!

@mvlassis mvlassis dismissed DnPlas’s stale review September 24, 2024 11:53

Comments addressed

@orfeas-k orfeas-k merged commit f1d42ec into main Sep 24, 2024
10 checks passed
@orfeas-k orfeas-k deleted the kf-6220-add-terraform-module branch September 24, 2024 11:54
orfeas-k added a commit that referenced this pull request Sep 24, 2024
Create a `terraform/` directory that hosts the Terraform module for this charm. It follows the structure proposed in [this spec](https://docs.google.com/document/d/1EG71A2pJ244PQRaGVzGj7Mx2B_bzE4U_OSqx4eeVI1E/edit) and it is follows what was done in canonical/argo-operators#198.

Ref #197
orfeas-k added a commit that referenced this pull request Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants