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

cli: common backend for init and upgrade apply commands #2449

Merged
merged 9 commits into from
Oct 24, 2023
Merged

cli: common backend for init and upgrade apply commands #2449

merged 9 commits into from
Oct 24, 2023

Conversation

daniel-weisse
Copy link
Member

@daniel-weisse daniel-weisse commented Oct 12, 2023 •
edited
Loading

Context

Our CLI commands constellation init and constellation upgrade apply share a lot of code.
Additionally, we would like to eventually deprecate both commands in favor of a single constellation apply command.
To reduce code duplication and allow for a single command, this PR aims to provide a single backend for both commands.
This backend should take care of initializing the cluster and/or applying upgrades.

Proposed change(s)

  • Add a common apply backend that handles the logic for both constellation init and constellation upgrade apply
  • Fix an issue with the ExtendClusterConfigCertSANs that would add an empty string to the clusters apiserver SANs field

Additional info

TODO

Checklist

  • Add labels (e.g., for changelog category)
  • Is PR title adequate for changelog?
  • Link to Milestone

@daniel-weisse daniel-weisse force-pushed the feat/cli/apply-backend branch from d4c0e19 to fd19bc9 Compare October 12, 2023 14:25
@daniel-weisse daniel-weisse force-pushed the feat/cli/apply-backend branch from fd19bc9 to d1a59c4 Compare October 13, 2023 07:56
@daniel-weisse daniel-weisse force-pushed the feat/cli/apply-backend branch 3 times, most recently from 917b5d0 to 18f8e9b Compare October 13, 2023 10:08
@daniel-weisse daniel-weisse added the no changelog Change won't be listed in release changelog label Oct 13, 2023
@daniel-weisse daniel-weisse force-pushed the feat/cli/apply-backend branch from 4ab82c4 to ad661fd Compare October 13, 2023 14:27
@daniel-weisse daniel-weisse force-pushed the feat/cli/apply-backend branch from ad661fd to c1925ba Compare October 16, 2023 06:20
@daniel-weisse daniel-weisse force-pushed the feat/cli/apply-backend branch from c1925ba to 2d77f54 Compare October 16, 2023 08:00
@daniel-weisse daniel-weisse force-pushed the feat/cli/apply-backend branch from 2d77f54 to 22a4441 Compare October 16, 2023 08:47
@daniel-weisse daniel-weisse temporarily deployed to e2e October 16, 2023 08:56 — with GitHub Actions Inactive
@daniel-weisse daniel-weisse mentioned this pull request Oct 16, 2023
Merged
3 tasks
Base automatically changed from ref/cli/flag-parsing to main October 16, 2023 13:05
@daniel-weisse daniel-weisse force-pushed the feat/cli/apply-backend branch from 22a4441 to d4467d3 Compare October 16, 2023 13:06
@netlify
Copy link

netlify bot commented Oct 16, 2023 •
edited
Loading

✅ Deploy Preview for constellation-docs ready!

Name Link
🔨 Latest commit b082669
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/6537c6126d318d00084559ab
😎 Deploy Preview https://deploy-preview-2449--constellation-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@daniel-weisse daniel-weisse marked this pull request as ready for review October 16, 2023 13:23
@daniel-weisse daniel-weisse added this to the v2.13.0 milestone Oct 17, 2023
@malt3 malt3 self-requested a review October 17, 2023 12:08
malt3
malt3 approved these changes Oct 17, 2023
View reviewed changes
Copy link
Contributor

@malt3 malt3 left a comment

Choose a reason for hiding this comment

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

I'm not that familiar with the CLI logic anymore but from my understanding, this looks very good.

cli/internal/cmd/apply.go Outdated Show resolved Hide resolved
@elchead elchead self-requested a review October 18, 2023 07:09
elchead
elchead reviewed Oct 18, 2023
View reviewed changes
cli/internal/cmd/apply.go Outdated Show resolved Hide resolved
elchead
elchead reviewed Oct 18, 2023
View reviewed changes
cli/internal/cmd/apply.go Outdated Show resolved Hide resolved
@elchead
Copy link
Contributor

elchead commented Oct 18, 2023

great work! lgtm. We should run init upgrade mini E2E tests

@daniel-weisse
Copy link
Member Author

We should run init upgrade mini E2E tests

Already done, see PR description for links

@daniel-weisse daniel-weisse mentioned this pull request Oct 20, 2023
Merged
4 tasks
@daniel-weisse daniel-weisse force-pushed the feat/cli/apply-backend branch from 7f42631 to c522607 Compare October 23, 2023 08:41
derpsteb
derpsteb approved these changes Oct 24, 2023
View reviewed changes
Copy link
Member

@derpsteb derpsteb left a comment

Choose a reason for hiding this comment

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

Sorry, I am not getting around to reviewing this more in depth. I think others already did so. Looks great on first sight.

daniel-weisse and others added 9 commits October 24, 2023 15:26
@daniel-weisse
Use common 'apply' backend for init and upgrades
62ef51d 
Signed-off-by: Daniel Weiße <[email protected]>
@daniel-weisse
Move unit tests to new apply backend
560b490 
Signed-off-by: Daniel Weiße <[email protected]>
@daniel-weisse
Split apply code into multiple files
5e7c9c9 
Signed-off-by: Daniel Weiße <[email protected]>
@msanft @daniel-weisse
cli: only perform Terraform migrations if state exists in cwd (#2457)
a17e231 
Signed-off-by: Moritz Sanft <[email protected]>
@daniel-weisse
Fix doc comment formatting
0e8774f 
Signed-off-by: Daniel Weiße <[email protected]>
@daniel-weisse
Update function names
70da886 
Signed-off-by: Daniel Weiße <[email protected]>
@daniel-weisse
Move validation logic to own function
0a8e145 
Signed-off-by: Daniel Weiße <[email protected]>
@daniel-weisse
Use skipPhases flag for propagating validation checks
86df9d8 
Signed-off-by: Daniel Weiße <[email protected]>
@daniel-weisse
Rework skipPhases logic
b082669 
Signed-off-by: Daniel Weiße <[email protected]>
@daniel-weisse daniel-weisse force-pushed the feat/cli/apply-backend branch from c522607 to b082669 Compare October 24, 2023 13:26
@github-actions
Copy link
Contributor

Coverage report

Package Old New Trend
cli/internal/cloudcmd 63.80% 65.00% ↗️
cli/internal/cmd 50.60% 52.80% ↗️
cli/internal/kubecmd 65.00% 64.00% ↘️
internal/config 80.20% 80.20% ↔️

@daniel-weisse daniel-weisse merged commit 671cf36 into main Oct 24, 2023
7 checks passed
@daniel-weisse daniel-weisse deleted the feat/cli/apply-backend branch October 24, 2023 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elchead elchead elchead approved these changes

@derpsteb derpsteb derpsteb approved these changes

@malt3 malt3 malt3 approved these changes

Assignees
No one assigned
Labels
no changelog Change won't be listed in release changelog
Projects
None yet
Milestone
v2.13.0
Development

Successfully merging this pull request may close these issues.
5 participants
@daniel-weisse @elchead @malt3 @derpsteb @msanft