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

Integrate istio ingress #1672

Merged
merged 2 commits into from
Sep 15, 2024
Merged

Conversation

abaguas
Copy link
Collaborator

@abaguas abaguas commented Jul 29, 2024

Fixes #552 The K8GB controller can now speak istio 🎉

User config

In #1557 the groundwork was done to be able to reference any ingress type.
The only implementation so far was Kubernetes Ingress resources. With this PR we bring to life the second implementation without touching any of the core logic: istio virtual services. The configuration looks as follows:

spec:
  resourceRef:
    apiVersion: networking.istio.io/v1
    kind: VirtualService
    matchLabels:
      app: istio

CRD changes

The previous API required CRD changes on every new integration because it was depending on a key with its name:

spec:
  resourceRef:
     istio:
        ...
     ingress:
        ...

To keep the CRD stable the structure is changed to:

spec:
  resourceRef:
    apiVersion: foo
    kind: bar

How istio ingress works

Istio uses istio ingress gateways for ingress. The users send traffic to the gateway, and then the gateway forward traffic to the application.
The ingress gateway is configured as follows:

  • Gateway resources configure the ports open on the gateway. This resource contains also a list of hosts (Host header or SNI) that it expects on that port.
  • Virtual Service resources configure to which kubernetes service the traffic should be routed, after it has been accepted by the ingress gateway. It contains a list of hosts (Host header or SNI), the service it should route the traffic to, and a reference to a Gateway resource.

How the k8gb controller integrates with istio

The k8gb controller is interested in three pieces of information: the domain of the endpoint and the IP address of the cluster ingress to create DNS entries; the kubernetes service in front of the application to monitor the health of the endpoints.
From the VirtualService resource all this information can be fetched, that is why this is the resource referenced by the GSLB. The domain and service are immediately available on the VirtualService resource. The IP address needs to be fetched from the istio ingress controller's LoadBalancer service which can reached by following -> VirtualService.spec.gateways -> Gateway.spec.selector.

Playground setup

To showcase this new integration the playground was extended with an istio ingress controller running side by side with the nginx ingress controller. The nginx ingress controller is still exposed on ports 80 and 81 (for clusters k3d-test-gslb1 and k3d-test-gslb2 respectively), and the istio ingress controller is exposed on ports 8080 and 8081.
In addition, since the podinfo pod behind the istio ingress requires a sidecar, a new namespace test-gslb-ingress was created to host this application and the respective GSLB resources.

Helm chart

The helm chart is adapted with a feature flag to enable the necessary RBAC read permissions.

Testing

These changes are backed by unit and e2e tests. The framework needed to be slightly adapted to support applying also VirtualService and Gateway resources but the core logic of both the frameworks and the tests remains.
In addition, a workflow was created to run the terratest suit for the v1beta1 apiVersion of istio, since we plan to support both v1beta1 and v1.
The PR looks big, but most of the changes are new manifests for the e2e testing setup.

@abaguas abaguas marked this pull request as draft July 29, 2024 20:56
Copy link

netlify bot commented Jul 29, 2024

Deploy Preview for k8gb-preview ready!

Name Link
🔨 Latest commit c765b3c
🔍 Latest deploy log https://app.netlify.com/sites/k8gb-preview/deploys/66dc1ed9f5f8e60008a9592c
😎 Deploy Preview https://deploy-preview-1672--k8gb-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@abaguas abaguas force-pushed the istio/virtualservice branch 20 times, most recently from de9630e to 0dc6a4f Compare August 5, 2024 13:59
@abaguas abaguas force-pushed the istio/virtualservice branch 2 times, most recently from 8910d6f to c18134d Compare August 11, 2024 11:58
@abaguas abaguas marked this pull request as ready for review August 11, 2024 12:37
@abaguas abaguas added istio Trigger terratest for all supported apiVersions and removed istio Trigger terratest for all supported apiVersions labels Sep 7, 2024
@abaguas abaguas added istio Trigger terratest for all supported apiVersions and removed istio Trigger terratest for all supported apiVersions labels Sep 7, 2024
@abaguas abaguas added istio Trigger terratest for all supported apiVersions and removed istio Trigger terratest for all supported apiVersions labels Sep 7, 2024
@abaguas abaguas added istio Trigger terratest for all supported apiVersions and removed istio Trigger terratest for all supported apiVersions labels Sep 7, 2024
@abaguas abaguas added istio Trigger terratest for all supported apiVersions and removed istio Trigger terratest for all supported apiVersions labels Sep 7, 2024
@abaguas abaguas added istio Trigger terratest for all supported apiVersions and removed istio Trigger terratest for all supported apiVersions labels Sep 7, 2024
@abaguas abaguas requested a review from ytsarev September 7, 2024 13:03
Signed-off-by: Andre Baptista Aguas <andre.aguas@protonmail.com>
ytsarev
ytsarev previously approved these changes Sep 15, 2024
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

I've updated the default-full-local-setup exaples to use new resourceRef style in 08623be

Also resolved the conflict in GHA workflow to be able to merge with master

k -n test-gslb get gslb failover-ingress -o yaml
apiVersion: k8gb.absa.oss/v1beta1
kind: Gslb
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"k8gb.absa.oss/v1beta1","kind":"Gslb","metadata":{"annotations":{},"name":"failover-ingress","namespace":"test-gslb"},"spec":{"resourceRef":{"apiVersion":"networking.k8s.io/v1","kind":"Ingress","matchLabels":{"app":"test-gslb-failover"}},"strategy":{"primaryGeoTag":"eu","type":"failover"}}}
  creationTimestamp: "2024-09-15T21:48:05Z"
  finalizers:
  - k8gb.absa.oss/finalizer
  generation: 2
  name: failover-ingress
  namespace: test-gslb
  resourceVersion: "1038"
  uid: abf12c92-0490-453d-bf1f-75194962a0aa
spec:
  ingress: {}
  resourceRef:
    apiVersion: networking.k8s.io/v1
    kind: Ingress
    matchLabels:
      app: test-gslb-failover
  strategy:
    dnsTtlSeconds: 30
    primaryGeoTag: eu
    splitBrainThresholdSeconds: 300
    type: failover
status:
  geoTag: us
  healthyRecords:
    failover.cloud.example.com:
    - 172.20.0.6
    - 172.20.0.7
  hosts: failover.cloud.example.com
  loadBalancer:
    exposedIps:
    - 172.20.0.10
    - 172.20.0.11
  servers:
  - host: failover.cloud.example.com
    services:
    - name: frontend-podinfo
      namespace: test-gslb
  serviceHealth:
    failover.cloud.example.com: Healthy

Local test with new style resourceRef looks great.

I've noticed that if we reference non-existing apiGroup or Kind , the Gslb will be silent and will not give the user any hints that it can't resolve resource and how to fix the situation

cat test-broken.yaml
apiVersion: k8gb.absa.oss/v1beta1
kind: Gslb
metadata:
  name: roundrobin-ingress-broken
  namespace: test-gslb
spec:
  resourceRef:
    apiVersion: networking.k8s.io/v1
    kind: Degress
    matchLabels:
      app: roundrobin-test-gslb
  strategy:
    type: roundRobin # Use a round robin load balancing strategy, when deciding which downstream clusters to route clients to
    splitBrainThresholdSeconds: 300 # Threshold after which external cluster is filtered out from delegated zone when it doesn't look alive
    dnsTtlSeconds: 30 # TTL value for automatically created DNS records

The Event stream will be empty

k describe -f test-broken.yaml
Name:         roundrobin-ingress-broken
Namespace:    test-gslb
Labels:       <none>
Annotations:  <none>
API Version:  k8gb.absa.oss/v1beta1
Kind:         Gslb
Metadata:
  Creation Timestamp:  2024-09-15T22:04:22Z
  Finalizers:
    k8gb.absa.oss/finalizer
  Generation:  2
  Managed Fields:
    API Version:  k8gb.absa.oss/v1beta1
    Fields Type:  FieldsV1
    fieldsV1:
      f:metadata:
        f:finalizers:
          .:
          v:"k8gb.absa.oss/finalizer":
      f:spec:
        f:ingress:
    Manager:      k8gb
    Operation:    Update
    Time:         2024-09-15T22:04:22Z
    API Version:  k8gb.absa.oss/v1beta1
    Fields Type:  FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .:
          f:kubectl.kubernetes.io/last-applied-configuration:
      f:spec:
        .:
        f:resourceRef:
        f:strategy:
          .:
          f:dnsTtlSeconds:
          f:splitBrainThresholdSeconds:
          f:type:
    Manager:         kubectl-client-side-apply
    Operation:       Update
    Time:            2024-09-15T22:04:22Z
  Resource Version:  2147
  UID:               e14fd3fb-0374-4f92-9fcf-73b1641e6d1a
Spec:
  Ingress:
  Resource Ref:
    API Version:  networking.k8s.io/v1
    Kind:         Degress
    Match Labels:
      App:  roundrobin-test-gslb
  Strategy:
    Dns Ttl Seconds:                30
    Split Brain Threshold Seconds:  300
    Type:                           roundRobin
Events:                             <none>

I suggest to create separate issue to track the verbosity enhancement and not bloat this PR

Approved, @abaguas thanks for the incredible work 🚀

Signed-off-by: Yury Tsarev <yury@upbound.io>
@ytsarev ytsarev merged commit 136f0d7 into k8gb-io:master Sep 15, 2024
14 checks passed
abaguas added a commit to abaguas/k8gb that referenced this pull request Oct 31, 2024
In the [PR](k8gb-io#1672 (review)) where the istio ingress integration was introduced, it was suggested to record events if GSLB references cannot be resolved. The controller was already generating logs, but an event would help the users debugging using `kubectl describe`.
This is a new feature that was not yet present in K8GB. Therefore, to implement it we need to fetch an EventRecorder and grant the controller the appropriate permissions. The error messages were also adapted and currently look like:
```
[k3d-test-gslb2|test-gslb] ➜ k describe gslb roundrobin-ingress-broken
...
Events:
  Type     Reason          Age                From             Message
  ----     ------          ----               ----             -------
  Warning  ReconcileError  2s (x12 over 12s)  gslb-controller  error resolving references (APIVersion:networking.k8s.io/v1, Kind:Degress not supported)
```
```
[k3d-test-gslb2|test-gslb] ➜ k get events
LAST SEEN   TYPE      REASON           OBJECT                           MESSAGE
5s          Warning   ReconcileError   gslb/roundrobin-ingress-broken   error resolving references (APIVersion:networking.k8s.io/v1, Kind:Degress not supported)
```

The controller also logs a message (this was already the case before this PR):
```
2024-10-31T16:47:29Z ERR github.com/k8gb-io/k8gb/controllers/logging/logr.go:72 > events: Reconciler error {"Gslb":"test-gslb/roundrobin-ingress-broken","controller":"gslb","controllerGroup":"k8gb.absa.oss","controllerKind":"Gslb","name":"roundrobin-ingress-broken","namespace":"test-gslb","reconcileID":"2a6a4ad5-96f6-4ee6-8c04-28de0016fc57"} error="error resolving references (APIVersion:networking.k8s.io/v1, Kind:Degress not supported)"
```

There are many other situations where we can generate events, but they won't be included in this PR to avoid bloating it.

[Documentation about events using kubebuilder](https://book-v1.book.kubebuilder.io/beyond_basics/creating_events)

---

This PR also upgrades the golinter since the old version was not compatible with my local go version. Upon upgrading I also resolved the following warning:
```
WARN The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar.
```

Signed-off-by: Andre Aguas <andre.aguas@protonmail.com>
abaguas added a commit that referenced this pull request Oct 31, 2024
In the [PR](#1672 (review)) where the istio ingress integration was introduced, it was suggested to record events if GSLB references cannot be resolved. The controller was already generating logs, but an event would help the users debugging using `kubectl describe`.
This is a new feature that was not yet present in K8GB. Therefore, to implement it we need to fetch an EventRecorder and grant the controller the appropriate permissions. The error messages were also adapted and currently look like:
```
[k3d-test-gslb2|test-gslb] ➜ k describe gslb roundrobin-ingress-broken
...
Events:
  Type     Reason          Age                From             Message
  ----     ------          ----               ----             -------
  Warning  ReconcileError  2s (x12 over 12s)  gslb-controller  error resolving references (APIVersion:networking.k8s.io/v1, Kind:Degress not supported)
```
```
[k3d-test-gslb2|test-gslb] ➜ k get events
LAST SEEN   TYPE      REASON           OBJECT                           MESSAGE
5s          Warning   ReconcileError   gslb/roundrobin-ingress-broken   error resolving references (APIVersion:networking.k8s.io/v1, Kind:Degress not supported)
```

The controller also logs a message (this was already the case before this PR):
```
2024-10-31T16:47:29Z ERR github.com/k8gb-io/k8gb/controllers/logging/logr.go:72 > events: Reconciler error {"Gslb":"test-gslb/roundrobin-ingress-broken","controller":"gslb","controllerGroup":"k8gb.absa.oss","controllerKind":"Gslb","name":"roundrobin-ingress-broken","namespace":"test-gslb","reconcileID":"2a6a4ad5-96f6-4ee6-8c04-28de0016fc57"} error="error resolving references (APIVersion:networking.k8s.io/v1, Kind:Degress not supported)"
```

There are many other situations where we can generate events, but they won't be included in this PR to avoid bloating it.

[Documentation about events using kubebuilder](https://book-v1.book.kubebuilder.io/beyond_basics/creating_events)

---

This PR also upgrades the golinter since the old version was not compatible with my local go version. Upon upgrading I also resolved the following warning:
```
WARN The linter 'exportloopref' is deprecated (since v1.60.2) due to: Since Go1.22 (loopvar) this linter is no longer relevant. Replaced by copyloopvar.
```

Signed-off-by: Andre Aguas <andre.aguas@protonmail.com>
@abaguas abaguas deleted the istio/virtualservice branch November 1, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
istio Trigger terratest for all supported apiVersions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request/Idea: Extend registration to ISTIO and Virtual Services
2 participants