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 Port discovery with multiple port services #267

Merged

Conversation

fcantournet
Copy link
Contributor

This fixes issue #263

We actually don't need to specify any ports in the VirtualService
and DestinationRules.
Istio will create clusters/listerners for each named port we have declared in
the kubernetes services and the router can be shared as it operates only on L7 criterias

Also contains a tiny clean-up of imports

k get canaries.flagger.app -o yaml places

    autoscalerRef:
      apiVersion: autoscaling/v2beta1
      kind: HorizontalPodAutoscaler
      name: places
    canaryAnalysis:
      interval: 5s
      maxWeight: 50
      metrics:
      - interval: 1m
        name: request-success-rate
        threshold: 99
      - interval: 1m
        name: request-duration
        threshold: 500
      stepWeight: 10
      threshold: 10
      webhooks:
      - metadata:
          cmd: test places --cleanup
          type: helm
        name: helm test
        timeout: 3m
        type: pre-rollout
        url: http://flagger-helmtester.kube-system/
    progressDeadlineSeconds: 60
    service:
      port: 8080
      portDiscovery: true
      trafficPolicy:
        tls:
          mode: DISABLE
    skipAnalysis: true
    targetRef:
      apiVersion: apps/v1
      kind: Deployment
      name: places

k get destinationrules.networking.istio.io -o yaml

- apiVersion: networking.istio.io/v1alpha3
  kind: DestinationRule
  metadata:
    creationTimestamp: "2019-07-23T16:30:35Z"
    generation: 1
    name: places-canary
    namespace: trip
    ownerReferences:
    - apiVersion: flagger.app/v1alpha3
      blockOwnerDeletion: true
      controller: true
      kind: Canary
      name: places
      uid: 30d633f1-ad67-11e9-a8c3-4201c0a8800a
    resourceVersion: "65133888"
    selfLink: /apis/networking.istio.io/v1alpha3/namespaces/trip/destinationrules/places-canary
    uid: 32f65ab0-ad67-11e9-8566-4201c0a8800b
  spec:
    host: places-canary
    trafficPolicy:
      tls:
        mode: DISABLE
- apiVersion: networking.istio.io/v1alpha3
  kind: DestinationRule
  metadata:
    creationTimestamp: "2019-07-23T16:30:35Z"
    generation: 1
    name: places-primary
    namespace: trip
    ownerReferences:
    - apiVersion: flagger.app/v1alpha3
      blockOwnerDeletion: true
      controller: true
      kind: Canary
      name: places
      uid: 30d633f1-ad67-11e9-a8c3-4201c0a8800a
    resourceVersion: "65133889"
    selfLink: /apis/networking.istio.io/v1alpha3/namespaces/trip/destinationrules/places-primary
    uid: 32f916f2-ad67-11e9-8566-4201c0a8800b
  spec:
    host: places-primary
    trafficPolicy:
      tls:
        mode: DISABLE

k get virtualservices.networking.istio.io -o yaml places

kind: VirtualService
metadata:
  creationTimestamp: "2019-07-23T16:30:36Z"
  generation: 6
  name: places
  namespace: trip
  ownerReferences:
  - apiVersion: flagger.app/v1alpha3
    blockOwnerDeletion: true
    controller: true
    kind: Canary
    name: places
    uid: 30d633f1-ad67-11e9-a8c3-4201c0a8800a
  resourceVersion: "70400304"
  selfLink: /apis/networking.istio.io/v1alpha3/namespaces/trip/virtualservices/places
  uid: 32faef14-ad67-11e9-8566-4201c0a8800b
spec:
  gateways:
  - mesh
  hosts:
  - places
  http:
  - route:
    - destination:
        host: places-primary
      weight: 100
    - destination:
        host: places-canary
      weight: 0

istioctl pc route $(k get pods -l app=synapse-trip -o jsonpath='{.items[0].metadata.name}') --name 8080 -o json

...
                "name": "places.trip.svc.cluster.local:8080",
                "domains": [
                    "places.trip.svc.cluster.local",
                    "places.trip.svc.cluster.local:8080",
                    "places",
                    "places:8080",
                    "places.trip.svc.cluster",
                    "places.trip.svc.cluster:8080",
                    "places.trip.svc",
                    "places.trip.svc:8080",
                    "places.trip",
                    "places.trip:8080",
                    "10.250.134.101",
                    "10.250.134.101:8080"
                ],
                "routes": [
                    {
                        "match": {
                            "prefix": "/"
                        },
                        "route": {
                            "cluster": "outbound|8080||places-primary.trip.svc.cluster.local",
...

istioctl pc route $(k get pods -l app=synapse-trip -o jsonpath='{.items[0].metadata.name}') --name 6565 -o json

...
                "name": "places.trip.svc.cluster.local:6565",
                "domains": [
                    "places.trip.svc.cluster.local",
                    "places.trip.svc.cluster.local:6565",
                    "places",
                    "places:6565",
                    "places.trip.svc.cluster",
                    "places.trip.svc.cluster:6565",
                    "places.trip.svc",
                    "places.trip.svc:6565",
                    "places.trip",
                    "places.trip:6565",
                    "10.250.134.101",
                    "10.250.134.101:6565"
                ],
                "routes": [
                    {
                        "match": {
                            "prefix": "/"
                        },
                        "route": {
                            "cluster": "outbound|6565||places-primary.trip.svc.cluster.local",
...

@stefanprodan
Copy link
Member

This looks great! Can you please change the FAQ and remove the but the virtual service will point to the port specified in spec.service.port.

Next step would be to turn on port discovery in the Istio e2e and add --port-metrics=9797 to podinfo. This way the e2e testing will be running with multiple ports and validate your PR.

This fixes issue fluxcd#263

We actually don't need to specify any ports in the VirtualService
and DestinationRules.
Istio will create clusters/listerners for each named port we have declared in
the kubernetes services and the router can be shared as it operates only on L7 criterias

Also contains a tiny clean-up of imports
@fcantournet fcantournet force-pushed the fix_virtualservices_multipleports branch from 3f29ff6 to 6651f64 Compare August 2, 2019 12:25
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @fcantournet

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.

2 participants