yorkie-team / yorkie

Yorkie is a document store for collaborative applications.
https://yorkie.dev
Apache License 2.0
761 stars 140 forks source link

Move Yorkie Related K8s Resources to Yorkie Namespace in Helm Chart #834

Closed krapie closed 4 months ago

krapie commented 4 months ago

What this PR does / why we need it?

This PR aims to move Yorkie-related Kubernetes resources from the istio-system namespace to the yorkie namespace within the yorkie-cluster Helm chart.

Previously, some yorkie-related resources with Istio dependencies were in the istio-system namespace. Moving them to the yorkie namespace is essential to explicitly isolate Yorkie-related resources from other services using Istio, preventing potential conflicts or confusion.

By relocating the Yorkie resources to the dedicated yorkie namespace, we ensure a clean separation of concerns and avoid namespace clutter caused by mixing Yorkie resources with other services using Istio.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

ExternalName typed Kubernetes Service has been added for Ingress object in yorkie namespace to communicate with istio-ingressgateway in istio-system namespace (which is in another namespace).

Also, ratelimit resources are not considered in this PR.

Does this PR introduce a user-facing change?:

Additional documentation:

Checklist:

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 50.95%. Comparing base (d5bc939) to head (342459c).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #834 +/- ## ======================================= Coverage 50.95% 50.95% ======================================= Files 70 70 Lines 10226 10226 ======================================= Hits 5211 5211 Misses 4487 4487 Partials 528 528 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hackerwins commented 4 months ago

NCP's ALB does not correctly support ExternalName

The setup in this PR involves mapping:

yorkie.yorkie<Ingress> <-> yorkie.istio-ingressgateway<externalname> <-> istio-system.istio-ingressgateway<service>

But NCP's Application LoadBalancer does not correctly support ExternalName. When trying to access an ExternalName service using NAVER Cloud Platform's Application LoadBalancer, the following error occurs:

cannot found nodeport: yorkie/istio-ingressgateway/servicePort:{0 80}

We'll roll back this PR, because the previous setup works correctly for the istio-system namespace.