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: Migration to new architecture #123

Merged
merged 91 commits into from
Aug 17, 2023

Conversation

dipankardas011
Copy link
Member

@dipankardas011 dipankardas011 commented Jul 7, 2023

Tasks description 🚧 🔧

Note
It is the parent PR from this Redesign architecture issue all other PR related to it should merge into this

### Issues
- [x] Closes #111
- [x] Closes #122
- [x] Closes #104
- [x] Closes #88
- [ ] Closes #87
- [ ] Closes #80
- [x] #125

Solution ✔️

Migration to the new Architecture design

Link for the latest architecture diagram https://www.figma.com/file/TM6aiNWMquYOxhCO23EKdT/ksctl?type=design&node-id=0%3A1&mode=design&t=Csg07pK02esAGQxU-1

Note to reviewers 📓

It should not be merged till the labels are assigned!
Before we merge this branch we need to update the CI as well
and increase the test coverage

  • Ran Unit Test locally
  • Tested the website locally

- added the basic look on the cmd part
- added the interface for the 3 controllers
- added the builder logic

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>

new emoji collections for the logging

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>

moved the create and delete subcommand to a 2 individual files

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>

improve: the branch to be more approachable for contributions

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>

started to restructure the api folder

- added the previous api and cmd in backup dir

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
@dipankardas011 dipankardas011 force-pushed the 111-feature-migration-to-new-architecture branch from 238b0e2 to 42c0161 Compare July 7, 2023 17:26
Signed-off-by: Dipankar Das <65275144+dipankardas011@users.noreply.github.com>
@dipankardas011 dipankardas011 changed the title 111 feature migration to new architecture feat: Migration to new architecture Jul 7, 2023
@dipankardas011 dipankardas011 added this to the New API Designing milestone Jul 7, 2023
ex. nomad

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Copy link
Contributor

@AvineshTripathi AvineshTripathi left a comment

Choose a reason for hiding this comment

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

With this review I have only looked into structuring, I've not seen into the inner working that I might see later but overall structure is currently my concern

api/provider/logger/noteMsg.go Outdated Show resolved Hide resolved
api/provider/utils/main_test.go Outdated Show resolved Hide resolved
api/provider/utils/main_test.go Outdated Show resolved Hide resolved
cli/cmd/createCluster.go Show resolved Hide resolved
cli/cmd/createCluster.go Outdated Show resolved Hide resolved
@dipankardas011
Copy link
Member Author

With this review I have only looked into structuring, I've not seen into the inner working that I might see later but overall structure is currently my concern

Let's have a call some day 🙏

@dipankardas011
Copy link
Member Author

With this review I have only looked into structuring, I've not seen into the inner working that I might see later but overall structure is currently my concern

for this review we will solve them one by one, once we get going, that's why not resolving these conversations

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
api/provider/local-bkp/main.go Outdated Show resolved Hide resolved
api/provider/logger/main.go Outdated Show resolved Hide resolved
api/provider/logger/noteMsg.go Outdated Show resolved Hide resolved
api/provider/logger/noteMsg.go Outdated Show resolved Hide resolved
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
as the functionallity is more coupled towards the cobracli we can move
it to a different place

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
@dipankardas011 dipankardas011 added kind/enhancement New feature or request cloud/civo cloud cloud/local local cloud/azure azure priority/high highest priority labels Jul 12, 2023
dipankardas011 and others added 5 commits August 14, 2023 09:30
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <65275144+dipankardas011@users.noreply.github.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Merging #123 (bb971a1) into main (d223190) will increase coverage by 8.00%.
Report is 22 commits behind head on main.
The diff coverage is 12.70%.

❗ Current head bb971a1 differs from pull request most recent head 65bcaa6. Consider uploading reports for the commit 65bcaa6 to get more accurate results

@@            Coverage Diff            @@
##            main     #123      +/-   ##
=========================================
+ Coverage   4.88%   12.89%   +8.00%     
=========================================
  Files         17       25       +8     
  Lines       3152     3754     +602     
=========================================
+ Hits         154      484     +330     
- Misses      2990     3247     +257     
- Partials       8       23      +15     
Files Changed Coverage Δ
api/k8s_distro/k3s/k3scontroller.go 0.00% <0.00%> (ø)
api/provider/azure/firewall.go 0.00% <0.00%> (ø)
api/provider/azure/managed_main.go 0.00% <0.00%> (ø)
api/provider/azure/network.go 0.00% <0.00%> (ø)
api/provider/azure/ssh.go 0.00% <0.00%> (ø)
api/provider/azure/vm.go 0.00% <0.00%> (ø)
api/provider/civo/firewall.go 0.00% <0.00%> (ø)
api/provider/civo/managed_main.go 0.00% <0.00%> (ø)
api/provider/civo/network.go 0.00% <0.00%> (ø)
api/provider/civo/ssh.go 0.00% <0.00%> (ø)
... and 15 more

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@dipankardas011 dipankardas011 added pr/mergable This PR is waiting to get merged pr/lgtm This PR look good to the reviewer labels Aug 16, 2023
it was using uninitalized state variable
changed it to use the cluster name passed

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>

removal of fixme for k3s version

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>

upated the readme

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>

bug-fix(civo): managed kubernetes version not getting saved in cloud state

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>

removed region as required field in switch

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>

added flag for skipping approval step

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
feat(jenkinsfile): added

Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
Signed-off-by: Dipankar Das <dipankardas0115@gmail.com>
@dipankardas011 dipankardas011 force-pushed the 111-feature-migration-to-new-architecture branch from bb971a1 to 65bcaa6 Compare August 17, 2023 14:42
@dipankardas011
Copy link
Member Author

TODOs

  • ksctl core
  • Update docs
  • enable CI pipelines
  • Add test cases by simple function calls
  • add mocks by using interfaces

the mocks will be taken up after this PR

@dipankardas011 dipankardas011 merged commit 043ac99 into main Aug 17, 2023
15 checks passed
@dipankardas011 dipankardas011 deleted the 111-feature-migration-to-new-architecture branch January 7, 2024 10:15
dipankardas011 added a commit that referenced this pull request Jan 9, 2024
…w-architecture

feat: Migration to new architecture
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/internal cloud/azure azure cloud/civo cloud cloud/local local go Pull requests that update Go code kind/enhancement New feature or request pr/lgtm This PR look good to the reviewer pr/mergable This PR is waiting to get merged priority/high highest priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants