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

feat: update OCICluster webhook validations #77

Merged
merged 1 commit into from
May 2, 2022
Merged

feat: update OCICluster webhook validations #77

merged 1 commit into from
May 2, 2022

Conversation

joekr
Copy link
Member

@joekr joekr commented Apr 28, 2022 •
edited
Loading

What this PR does / why we need it:
This expands on the OCICluster webhook adding some more validation during cluster creation.

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 #70

@joekr joekr added the enhancement New feature or request label Apr 28, 2022
@joekr joekr self-assigned this Apr 28, 2022
@joekr joekr removed the request for review from Djelibeybi April 28, 2022 22:22
joekr
joekr commented Apr 28, 2022
View reviewed changes
api/v1beta1/validator.go Outdated Show resolved Hide resolved
@joekr joekr force-pushed the issue-70-cluster-validation branch from e928a89 to 184feed Compare April 28, 2022 22:26
@joekr
Copy link
Member Author

joekr commented Apr 28, 2022

unit

?       github.com/oracle/cluster-api-provider-oci      [no test files]
ok      github.com/oracle/cluster-api-provider-oci/api/v1beta1  0.508s  coverage: 16.1% 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.505s  coverage: 30.3% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/scope  175.925s        coverage: 81.9% 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/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  26.544s coverage: 71.0% of statements

e2e

Ran 5 of 21 Specs in 1985.456 seconds
SUCCESS! -- 5 Passed | 0 Failed | 0 Pending | 16 Skipped

@joekr joekr marked this pull request as ready for review April 28, 2022 23:53
shyamradhakrishnan
shyamradhakrishnan reviewed Apr 29, 2022
View reviewed changes
api/v1beta1/ocicluster_webhook.go Show resolved Hide resolved
api/v1beta1/validator.go Outdated Show resolved Hide resolved
api/v1beta1/validator.go Outdated Show resolved Hide resolved
api/v1beta1/validator.go Show resolved Hide resolved
api/v1beta1/validator.go Outdated Show resolved Hide resolved
api/v1beta1/validator.go Show resolved Hide resolved
api/v1beta1/ocicluster_webhook.go Outdated Show resolved Hide resolved
@joekr joekr marked this pull request as draft April 29, 2022 16:14
@joekr joekr force-pushed the issue-70-cluster-validation branch 4 times, most recently from 67ff03c to e169ff4 Compare April 29, 2022 22:02
shyamradhakrishnan
shyamradhakrishnan reviewed Apr 30, 2022
View reviewed changes
Copy link
Member

@shyamradhakrishnan shyamradhakrishnan left a comment

Choose a reason for hiding this comment

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

loog good, should we use logger rather than fmt.Println?

api/v1beta1/ocicluster_webhook.go Outdated Show resolved Hide resolved
@joekr joekr force-pushed the issue-70-cluster-validation branch from e169ff4 to ea086cd Compare May 1, 2022 00:17
@joekr joekr marked this pull request as ready for review May 1, 2022 00:21
shyamradhakrishnan
shyamradhakrishnan approved these changes May 2, 2022
View reviewed changes
Copy link
Member

@shyamradhakrishnan shyamradhakrishnan left a comment

Choose a reason for hiding this comment

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

Please run the PR blocking e2e tests before merging.

@joekr
Copy link
Member Author

joekr commented May 2, 2022

Updated

unit

?       github.com/oracle/cluster-api-provider-oci      [no test files]
ok      github.com/oracle/cluster-api-provider-oci/api/v1beta1  0.250s  coverage: 19.0% 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.481s  coverage: 27.8% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/scope  175.980s        coverage: 81.9% 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/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  26.334s coverage: 71.0% of statements

#e2e

Ran 5 of 21 Specs in 1827.671 seconds
SUCCESS! -- 5 Passed | 0 Failed | 0 Pending | 16 Skipped

@joekr joekr merged commit 096f2e0 into oracle:main May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shyamradhakrishnan shyamradhakrishnan shyamradhakrishnan approved these changes

Assignees

@joekr joekr

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add more validation for OCICluster
2 participants
@joekr @shyamradhakrishnan