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

inital pass at adding terraform for the k8s charms #194

Merged
merged 13 commits into from
Dec 16, 2024

Conversation

asbalderson
Copy link
Contributor

Applicable spec: https://docs.google.com/document/d/1EG71A2pJ244PQRaGVzGj7Mx2B_bzE4U_OSqx4eeVI1E/edit?tab=t.0

Overview

Adds basic terraform modules for the k8s charms

Rationale

Enable deploying canonical k8s with terraform

Juju Events Changes

N/A

Module Changes

N/A

Library Changes

N/A

Checklist

@asbalderson asbalderson marked this pull request as ready for review December 2, 2024 18:02
@asbalderson asbalderson requested a review from a team as a code owner December 2, 2024 18:02
Copy link
Contributor

@louiseschmidtgen louiseschmidtgen left a comment

Choose a reason for hiding this comment

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

Did a first pass on the templates, thank you so much for your work! I believe these should find a separate repo to live in.

charms/worker/k8s/terraform/README.md Outdated Show resolved Hide resolved
charms/worker/k8s/terraform/README.md Outdated Show resolved Hide resolved
charms/worker/k8s/terraform/variables.tf Outdated Show resolved Hide resolved
Copy link
Contributor Author

@asbalderson asbalderson left a comment

Choose a reason for hiding this comment

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

Adjusted from feedback

Also made some adjustments getting from the test deployment on prodstack like fixing the names of the endpoints

charms/worker/k8s/terraform/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@louiseschmidtgen louiseschmidtgen left a comment

Choose a reason for hiding this comment

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

Thanks @asbalderson for addressing my comments! Some more little nits and questions then we should be good to go.

Bonus points if you could add some module and functional tests. Else, we can create those later on.

charms/worker/k8s/terraform/README.md Outdated Show resolved Hide resolved
charms/worker/k8s/terraform/README.md Outdated Show resolved Hide resolved
charms/worker/k8s/terraform/README.md Show resolved Hide resolved
@addyess
Copy link
Contributor

addyess commented Dec 9, 2024

@asbalderson before you push another commit here, please pull to get in sync with this branch

Copy link
Contributor Author

@asbalderson asbalderson left a comment

Choose a reason for hiding this comment

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

Comments from review

charms/worker/k8s/terraform/README.md Show resolved Hide resolved
Copy link
Contributor

@louiseschmidtgen louiseschmidtgen left a comment

Choose a reason for hiding this comment

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

LGTM, one little nit to the Readme. Confirmed things are deploying on my end as well.
Awesome work, thank you so much!

charms/worker/k8s/terraform/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

Test coverage for 4c2de99

coverage-report: install_deps /home/runner/work/k8s-operator/k8s-operator/charms/worker/k8s> python -I -m pip install 'coverage[toml]'
coverage-report: commands[0] /home/runner/work/k8s-operator/k8s-operator/charms/worker/k8s> coverage report
Name                                    Stmts   Miss  Cover
-----------------------------------------------------------
lib/charms/k8s/v0/k8sd_api_manager.py     278     29    90%
src/charm.py                              497    250    50%
src/cloud_integration.py                   80      3    96%
src/config/extra_args.py                   29      2    93%
src/containerd.py                         140     16    89%
src/cos_integration.py                     33     12    64%
src/events/update_status.py                51     10    80%
src/inspector.py                           40      4    90%
src/kube_control.py                        39     31    21%
src/literals.py                            21      0   100%
src/protocols.py                           26      5    81%
src/reschedule.py                          77      4    95%
src/snap.py                               185     26    86%
src/token_distributor.py                  181    109    40%
src/upgrade.py                            105     48    54%
-----------------------------------------------------------
TOTAL                                    1782    549    69%
coverage-report: OK (1.24=setup[1.02]+cmd[0.22] seconds)
congratulations :) (1.29 seconds)

Static code analysis report

Run started:2024-12-13 20:10:18.902288

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 3818
  Total lines skipped (#nosec): 3
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@louiseschmidtgen louiseschmidtgen merged commit f83d7c5 into canonical:main Dec 16, 2024
59 checks passed
@asbalderson asbalderson deleted the k8s-terraform branch December 16, 2024 18:55
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