-
Notifications
You must be signed in to change notification settings - Fork 8.2k
[WIP] v1alpha2 conversion tool #3822
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
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
Potential things to expand on:
|
|
@ZackButcher: I'd appreciate a quick scan if you have the time |
|
@ijsnellf can you expand on the description please? This is actually pretty cool. |
Codecov Report
@@ Coverage Diff @@
## master #3822 +/- ##
=======================================
- Coverage 76% 76% -<1%
=======================================
Files 291 295 +4
Lines 26939 27419 +480
=======================================
+ Hits 20426 20630 +204
- Misses 5205 5440 +235
- Partials 1308 1349 +41
Continue to review full report at Codecov.
|
|
@rshriram: I added a description. Please let me know if it makes sense. |
|
@ijsnellf: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@ijsnellf PR needs rebase |
ZackButcher
left a comment
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.
Very cool, and exciting to have this tooling before we adopt the new APIs!
| out = append(out, model.Config{ | ||
| ConfigMeta: model.ConfigMeta{ | ||
| Type: model.V1alpha2RouteRule.Type, | ||
| Name: "FIXME", |
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.
host, since we're aggregating by host
| Type: model.V1alpha2RouteRule.Type, | ||
| Name: "FIXME", | ||
| Namespace: "FIXME", | ||
| Domain: "FIXME", |
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.
I think to be correct we'd have to create a route rule per (host, namespace, domain), instead of just host. However, given the nature of the tool, I think it'd also be fair to just pick the namespace + domain of one of the source DestinationPolicies (e.g. the first we touch for that host) and print a warning to STDERR saying it merged rules across namespaces and advising to only process files with objects belonging to a single namespace at a time.
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.
That's a good suggestion, thanks
| func convertConfigs(readers []io.Reader, writer io.Writer) error { | ||
| configDescriptor := model.ConfigDescriptor{ | ||
| model.RouteRule, | ||
| model.V1alpha2RouteRule, |
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.
update for VirtualService rename
| func Command() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "convert", | ||
| Short: "Convert configs from v1alpha1 to v1alpha2", |
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.
"Convert configs from v1alpha1 to v1alpha2" -> "Convert configs from v1alpha1 to v1alpha3"
| cmd := &cobra.Command{ | ||
| Use: "convert", | ||
| Short: "Convert configs from v1alpha1 to v1alpha2", | ||
| Long: "Converts sets of v1alpha1 configs to v1alpha2 equivalents on a best effort basis. " + |
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.
v1alpha3
| Use: "convert", | ||
| Short: "Convert configs from v1alpha1 to v1alpha2", | ||
| Long: "Converts sets of v1alpha1 configs to v1alpha2 equivalents on a best effort basis. " + | ||
| "The output should be considered a starting point for your v1alpha2 configs and probably " + |
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.
v1alpha3
| case model.RouteRule.Type: | ||
| err = model.ValidateRouteRule(config.Spec) | ||
| case model.V1alpha2RouteRule.Type: | ||
| err = model.ValidateRouteRuleV2(config.Spec) |
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.
VirtualService name change
| "github.com/golang/protobuf/ptypes/duration" | ||
|
|
||
| routing "istio.io/api/routing/v1alpha1" | ||
| routingv2 "istio.io/api/routing/v1alpha2" |
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.
import rename
| @@ -0,0 +1,24 @@ | |||
| apiVersion: config.istio.io/v1alpha2 | |||
| kind: V1alpha2RouteRule | |||
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.
VirtualService
| @@ -0,0 +1,18 @@ | |||
| apiVersion: config.istio.io/v1alpha2 | |||
| kind: V1alpha2RouteRule | |||
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.
VirtualService
| @@ -0,0 +1,29 @@ | |||
| apiVersion: config.istio.io/v1alpha2 | |||
| kind: V1alpha2RouteRule | |||
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.
VirtualService
| @@ -0,0 +1,14 @@ | |||
| apiVersion: config.istio.io/v1alpha2 | |||
| kind: V1alpha2RouteRule | |||
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.
VirtualService
| readers := make([]io.Reader, 0) | ||
| for _, filename := range inFilenames { | ||
| if filename == "-" { | ||
| readers = append(readers, os.Stdin) |
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.
TODO: set readers and break here
|
Closing in favor of #4308 |
Converts multiple v1alpha1 configs into equivalent v1alpha2 configs.
edit:
From the command summary: