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

Add support for ServiceSplitter #339

Merged
merged 5 commits into from
Sep 29, 2020
Merged

Add support for ServiceSplitter #339

merged 5 commits into from
Sep 29, 2020

Conversation

ishustava
Copy link
Contributor

@ishustava ishustava commented Sep 28, 2020 •
edited
Loading

Depends on #332

Changes proposed in this PR:

  • Add CRD support for the service splitter
  • Update the version of controller-tools to 0.4.0 to support float types. With this version, we can pass allowDangerousTypes marker to allow CRDs to have float32 types. This also comes with a breaking change to CRD and webhook versions, where now we have to explicitly set versions to v1beta1 since controller-tools now defaults to v1.

How I've tested this PR:
Acceptance tests in hashicorp/consul-helm#624

How I expect reviewers to test this PR:
👀 acceptance tests in hashicorp/consul-helm#624

Checklist:

  • Tests added

thisisnotashwin
thisisnotashwin reviewed Sep 29, 2020
View reviewed changes
Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

This looks phenomenal. Left a couple of suggestions but im still on the fence about the situation with % signs.

api/v1alpha1/servicesplitter_types.go Outdated Show resolved Hide resolved
api/v1alpha1/servicesplitter_types.go Outdated Show resolved Hide resolved
api/v1alpha1/servicesplitter_types.go Outdated Show resolved Hide resolved
api/v1alpha1/servicesplitter_types_test.go Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
subcommand/controller/command.go Outdated Show resolved Hide resolved
Base automatically changed from crd-controller-router to crd-controller-base September 29, 2020 16:35
lkysow
lkysow reviewed Sep 29, 2020
View reviewed changes
api/v1alpha1/servicesplitter_types.go Outdated Show resolved Hide resolved
api/v1alpha1/servicesplitter_types.go Outdated Show resolved Hide resolved
controllers/configentry_controller_test.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
subcommand/controller/command.go Outdated Show resolved Hide resolved
thisisnotashwin
thisisnotashwin approved these changes Sep 29, 2020
View reviewed changes
Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

🏅

@ishustava ishustava requested a review from lkysow September 29, 2020 20:14
lkysow
lkysow approved these changes Sep 29, 2020
View reviewed changes
Copy link
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

Awesome!

@ishustava ishustava merged commit 423cbac into crd-controller-base Sep 29, 2020
@ishustava ishustava deleted the crd-service-splitter branch September 29, 2020 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thisisnotashwin thisisnotashwin thisisnotashwin approved these changes

@lkysow lkysow lkysow approved these changes

Assignees
No one assigned
Labels
theme/crds type/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ishustava @lkysow @thisisnotashwin