-
Notifications
You must be signed in to change notification settings - Fork 37
Support routed networks #439
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
Support routed networks #439
Conversation
✅ Deploy Preview for kubernetes-sigs-cluster-api-cloudstack ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
89c72e5
to
4847bbe
Compare
4847bbe
to
55bdc5d
Compare
/assign @weizhouapache |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #439 +/- ##
==========================================
+ Coverage 25.66% 25.78% +0.11%
==========================================
Files 59 72 +13
Lines 5563 6778 +1215
==========================================
+ Hits 1428 1748 +320
- Misses 3996 4865 +869
- Partials 139 165 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vishesh92
great job !
I left two minor comments
Setting up environment failed |
fbe2232
to
49df11a
Compare
Setting up environment failed |
Test Results : (tid-857)
|
Test Results : (tid-858)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vishesh92, weizhouapache The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
49df11a
to
08d655a
Compare
/lgtm |
@vishesh92 |
08d655a
to
5674a5c
Compare
Yes. I think it's due to golangci-lint & go version. |
5674a5c
to
fcba507
Compare
fcba507
to
4aec197
Compare
/lgtm |
Test Results : (tid-859)
|
Issue #, if available:
Fixes #435
Description of changes:
Generated
This pull request introduces several updates to the CloudStack provider, including enhancements to network configuration, dependency upgrades, and validation logic improvements. The changes also include a migration to the Uber
mock
library for mocking in tests. Below are the most important changes grouped by theme:Network Configuration Enhancements:
NetworkMode
field to theNetwork
andCloudStackIsolatedNetworkStatus
structs to support routing mode configurations. This field is also reflected in the CRDs (cloudstackclusters.yaml
,cloudstackfailuredomains.yaml
,cloudstackisolatednetworks.yaml
). [1] [2] [3] [4] [5]Network()
method ofCloudStackIsolatedNetwork
to include theNetworkMode
from its status.NetworkMode
in theAddToLBIfNeeded
method, ensuring that load balancer assignment is skipped for routed networks.Validation Logic Improvements:
ValidateUpdate
method inCloudStackCluster
to allow updates to theControlPlaneEndpoint.Host
field while ensuring thePort
remains unchanged.Dependency and Tooling Updates:
github.com/golang/mock
togo.uber.org/mock
for mocking in tests, updating all relevant test files and theMakefile
. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]go.mod
files, includingcloudstack-go
to v2.17.0 andgolangci-lint
to v1.60.2. [1] [2] [3] [4] [5] [6]These changes improve the flexibility of network configurations, enhance validation rules, and modernize the codebase with updated dependencies and tooling.
Testing performed:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.