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

Fix merge strategy for NSG/Subnet and remove usage of ID for NSG secl… #116

Conversation

shyamradhakrishnan
Copy link
Contributor

Fix merge strategy for NSG/Subnet and remove usage of ID for NSG seclist reconciliation

What this PR does / why we need it:

  • NSG/Subnet lists are hitting the issue mentioned https://main.cluster-api.sigs.k8s.io/developer/providers/v1.1-to-v1.2.html#required-api-changes-for-providers . Fix is as per the suggestion in the doc to make name as the map key and make it mandatory.
  • NSG Security rules has a similar problem but cant be fixed in a similar way because ID is generated and hence cannot be used in ClusterClass Server side patching. The ClusterClass template will not have the ID. Also, ideally, the NSG Seclist rules should just be based on the actual rule comparison and not ID comparison. So we have changed code to use rule comparison.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #113

@shyamradhakrishnan
Copy link
Contributor Author

All e2e test including non PR blocking run

Summarizing 1 Failure:

[Fail] Workload cluster creation [It] Oracle Linux - With 1 control-plane nodes and 1 worker nodes
/home/ubuntu/go/src/github.com/oracle/cluster-api-provider-oci/vendor/sigs.k8s.io/cluster-api/test/framework/controlplane_helpers.go:147

Ran 11 of 22 Specs in 3555.269 seconds
FAIL! -- 10 Passed | 1 Failed | 0 Pending | 11 Skipped


Ginkgo ran 1 suite in 59m19.12487375s
Test Suite Failed

Ginkgo 2.0 is coming soon!
==========================
Ginkgo 2.0 is under active development and will introduce several new features, improvements, and a small handful of breaking changes.
A release candidate for 2.0 is now available and 2.0 should GA in Fall 2021.  Please give the RC a try and send us feedback!
  - To learn more, view the migration guide at https://github.com/onsi/ginkgo/blob/ver2/docs/MIGRATING_TO_V2.md
  - For instructions on using the Release Candidate visit https://github.com/onsi/ginkgo/blob/ver2/docs/MIGRATING_TO_V2.md#using-the-beta
  - To comment, chime in at https://github.com/onsi/ginkgo/issues/711

To silence this notice, set the environment variable: ACK_GINKGO_RC=true
Alternatively you can: touch $HOME/.ack-ginkgo-rc
make[1]: *** [Makefile:258: test-e2e-run] Error 1
make[1]: Leaving directory '/home/ubuntu/go/src/github.com/oracle/cluster-api-provider-oci'
make: *** [Makefile:266: test-e2e] Error 2

Oracle Linux failed because of image problem.
Reran using correct image

• [SLOW TEST:562.755 seconds]
Workload cluster creation
/home/ubuntu/go/src/github.com/oracle/cluster-api-provider-oci/test/e2e/cluster_test.go:50
  Oracle Linux - With 1 control-plane nodes and 1 worker nodes
  /home/ubuntu/go/src/github.com/oracle/cluster-api-provider-oci/test/e2e/cluster_test.go:179
------------------------------
STEP: Tearing down the management cluster


Ran 1 of 22 Specs in 638.869 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 21 Skipped

@shyamradhakrishnan
Copy link
Contributor Author

unit tests

Using cached envtest tools from /Users/shyaradh/oke/src/github.com/oracle/cluster-api-provider-oci/testbin
setting up env vars
?   	github.com/oracle/cluster-api-provider-oci	[no test files]
ok  	github.com/oracle/cluster-api-provider-oci/api/v1beta1	0.739s	coverage: 18.7% of statements
?   	github.com/oracle/cluster-api-provider-oci/cloud/config	[no test files]
ok  	github.com/oracle/cluster-api-provider-oci/cloud/ociutil	0.439s	coverage: 25.0% of statements
ok  	github.com/oracle/cluster-api-provider-oci/cloud/scope	176.616s	coverage: 73.5% of statements
?   	github.com/oracle/cluster-api-provider-oci/cloud/scope/mocks	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/cloud/services/compute	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/cloud/services/compute/mock_compute	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/cloud/services/computemanagement	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/cloud/services/identity	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/cloud/services/identity/mock_identity	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer/mock_nlb	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/cloud/services/vcn	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/cloud/services/vcn/mock_vcn	[no test files]
ok  	github.com/oracle/cluster-api-provider-oci/controllers	27.378s	coverage: 71.0% of statements
?   	github.com/oracle/cluster-api-provider-oci/exp/api/v1beta1	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/exp/controllers	[no test files]
?   	github.com/oracle/cluster-api-provider-oci/feature	[no test files]

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.

Reconciliation is failing in ClusterClass is Custom NSG/Seclist is used
2 participants