BUG: Currently conversion tool changes namespace to default

[lordofthejars]

Is this a BUG or FEATURE REQUEST?: BUG

Did you review https://istio.io/help/ and existing issues to identify if this is already solved or being worked on?: Y

Bug: Y

What Version of Istio and Kubernetes are you using, where did you get Istio from, Installation details

istioctl 0.8.0
kubectl 1.9.1
openshift 3.9.0

Is Istio Auth enabled or not ? N

What happened: When I tried the conversion tool from an old routerule (simple one) to the new format, the namespace that was configured to old file was changed in the new file to default.

What you expected to happen: Namespace should be maintained and not changed

If you agree I can provide a PR fixing it, so the behaviour is if old file specifies a namespace then it uses, if not then default is used.

I know that this namespace can be configured with an argument but why not automatically?

How to reproduce it: Just make a simple conversion from one weight route described with old format containing a namespace to the new format.

apiVersion: config.istio.io/v1alpha2
kind: RouteRule
metadata:
  name: recommendation-default
spec:
  destination:
    namespace: tutorial
    name: recommendation
  precedence: 1
  route:
  - labels:
      version: v2
    weight: 100

[ymesika]

The sample you provided is indeed in the default namespace so should be left there.
However, the destination namespace is indeed not being converted. Is it what your fix is fixing?

If you plan to PR it (feel free to do so) please also include a test case to cover it.

[lordofthejars]

Yes exactly this is what I want to cover sorry, because the converted output is:

apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: recommendation
  namespace: default
spec:
  hosts:
  - recommendation
  http:
  - route:
    - destination:
        host: recommendation
        subset: version-v2
      weight: 100
---
apiVersion: networking.istio.io/v1alpha3
kind: DestinationRule
metadata:
  name: recommendation
  namespace: default
spec:
  host: recommendation
  subsets:
  - name: version-v1
    labels:
      version: v1
  - name: version-v2
    labels:
      version: v2

So I am wondering if in this case, the namespace should set correctly and avoid any default reference.

But I think it should be:

apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: recommendation
  namespace: tutorial
spec:
  hosts:
  - recommendation
  http:
  - route:
    - destination:
        host: recommendation
        subset: version-v2
      weight: 100
---
apiVersion: networking.istio.io/v1alpha3
kind: DestinationRule
metadata:
  name: recommendation
  namespace: tutorial
spec:
  host: recommendation
  subsets:
  - name: version-v1
    labels:
      version: v1
  - name: version-v2
    labels:
      version: v2

[lordofthejars]

@ymesika Do you agree with my purposed solution? In version alpha1 namespace could be set locally in destination but not now, so the only way I see is the one I have previously described. Starting coding a PR.

[ymesika]

There are actually two possible conversion directions here:

1. Mapping the namespace to the output configs namespaces [What you showed above]
2. Use the namespace from the input to construct a name.namespace host name

To keep it simple I would go with your suggested output if you are willing to contribute a fix.

@rshriram do you agree?

[ymesika]

CC @esnible who is working on this conversion tool.

[lordofthejars]

@esnible I have found other migration problems, for example when you are using mirroring creating two destinations which makes istio do not mirror the traffic, the mirrored service should not be in route destination section. But let's s going step by step, if you agree I can send PR fixing namespace problem.

[louiscryan]

Fixes would only go in 0.8 patch releases

https://github.com/istio/istio/issues/5982

[andraxylia]

Issue moved to istio/istio #6571 via ZenHub