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

Add possibility to specify services type per service #83

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

Sh4d1
Copy link
Contributor

@Sh4d1 Sh4d1 commented May 7, 2024

So my use case is to expose only the quickwit indexer through a load balancer. This patch is a bit more general but should get the work done. Haven't had time to test it yet!

charts/quickwit/values.yaml Outdated Show resolved Hide resolved
charts/quickwit/values.yaml Outdated Show resolved Hide resolved
charts/quickwit/values.yaml Outdated Show resolved Hide resolved
@Sh4d1
Copy link
Contributor Author

Sh4d1 commented Jun 10, 2024

sorry for the delay, it should be good now @rdettai !

@fmassot fmassot requested a review from idrissneumann June 24, 2024 11:03
charts/quickwit/values.yaml Outdated Show resolved Hide resolved
charts/quickwit/values.yaml Outdated Show resolved Hide resolved
charts/quickwit/values.yaml Outdated Show resolved Hide resolved
charts/quickwit/values.yaml Outdated Show resolved Hide resolved
charts/quickwit/values.yaml Outdated Show resolved Hide resolved
@idrissneumann
Copy link
Collaborator

Here's the result of some templating tests, seems okay ✅ :

---
config:
  default_index_root_uri: s3://qw-indexes
  storage:
    s3:
      endpoint: https://fr-par.scw.cloud
      region: fr-par
      access_key_id: SCWXXXXXXXX
      secret_access_key: XXXXXXX

environment:
  QW_METASTORE_URI: s3://qw-indexes

indexer:
  serviceType: NodePort

control_plane:
  serviceAnnotations:
    FOO: bar

Result, the annotation are here:

$ helm template . --values values.yaml --values values2.yaml|grep -i foo -B11
kind: Service
metadata:
  name: release-name-quickwit-control-plane
  labels:
    helm.sh/chart: quickwit-0.5.16
    app.kubernetes.io/name: quickwit
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "v0.8.1"
    app.kubernetes.io/managed-by: Helm
    
  annotations:
    FOO: bar

And the service type has changed for the indexer:

$ helm template . --values values.yaml --values values2.yaml|grep -i NodePort -B12
kind: Service
metadata:
  name: release-name-quickwit-indexer
  labels:
    helm.sh/chart: quickwit-0.5.16
    app.kubernetes.io/name: quickwit
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "v0.8.1"
    app.kubernetes.io/managed-by: Helm
    
  annotations:
spec:
  type: NodePort

Tests with kind are coming :)

@idrissneumann idrissneumann self-requested a review June 26, 2024 13:46
@idrissneumann
Copy link
Collaborator

idrissneumann commented Jun 26, 2024

Tests on kind:

$ kubectl create ns quickwit && helm template . --values values.yaml --values values2.yaml 
--namespace quickwit | kubectl -n quickwit apply -f - > /dev/null 2>&1

Result, the type of svc has changed for the indexer ✅

$ kubectl -n quickwit get svc
NAME                                  TYPE        CLUSTER-IP     EXTERNAL-IP   PORT(S)                         AGE
release-name-quickwit-control-plane   ClusterIP   10.96.5.107    <none>        7280/TCP,7281/TCP               38s
release-name-quickwit-headless        ClusterIP   None           <none>        7282/UDP,7280/TCP,7281/TCP      38s
release-name-quickwit-indexer         NodePort    10.96.225.37   <none>        7280:30205/TCP,7281:31836/TCP   38s
release-name-quickwit-janitor         ClusterIP   10.96.27.14    <none>        7280/TCP,7281/TCP               38s
release-name-quickwit-metastore       ClusterIP   10.96.184.30   <none>        7280/TCP,7281/TCP               38s
release-name-quickwit-searcher        ClusterIP   10.96.173.65   <none>        7280/TCP,7281/TCP               38s

And the annotations FOO: bar is present on the control_plane svc ✅ :

$ kubectl -n quickwit get svc release-name-quickwit-control-plane -o yaml
apiVersion: v1
kind: Service
metadata:
  annotations:
    FOO: bar
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"v1","kind":"Service","metadata":{"annotations":{"FOO":"bar"},"labels":{"app.kubernetes.io/instance":"release-name","app.kubernetes.io/managed-by":"Helm","app.kubernetes.io/name":"quickwit","app.kubernetes.io/version":"v0.8.1","helm.sh/chart":"quickwit-0.5.16"},"name":"release-name-quickwit-control-plane","namespace":"quickwit"},"spec":{"ports":[{"name":"rest","port":7280,"protocol":"TCP","targetPort":"rest"},{"name":"grpc","port":7281,"targetPort":"grpc"}],"selector":{"app.kubernetes.io/component":"control-plane","app.kubernetes.io/instance":"release-name","app.kubernetes.io/name":"quickwit"},"type":"ClusterIP"}}
  creationTimestamp: "2024-06-26T13:47:37Z"
  labels:
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: quickwit
    app.kubernetes.io/version: v0.8.1
    helm.sh/chart: quickwit-0.5.16
  name: release-name-quickwit-control-plane
  namespace: quickwit
  resourceVersion: "5297"
  uid: ec8c1a4a-1a47-4292-9dc8-a5d47032b6cb
spec:
  clusterIP: 10.96.5.107
  clusterIPs:
  - 10.96.5.107
  internalTrafficPolicy: Cluster
  ipFamilies:
  - IPv4
  ipFamilyPolicy: SingleStack
  ports:
  - name: rest
    port: 7280
    protocol: TCP
    targetPort: rest
  - name: grpc
    port: 7281
    protocol: TCP
    targetPort: grpc
  selector:
    app.kubernetes.io/component: control-plane
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/name: quickwit
  sessionAffinity: None
  type: ClusterIP
status:
  loadBalancer: {}

LGTM @Sh4d1 I think you just have to set the version in the Chart.yml to 0.6.1 :)

@Sh4d1
Copy link
Contributor Author

Sh4d1 commented Jun 26, 2024

@idrissneumann done w/ a rebase !

@idrissneumann idrissneumann merged commit 86f1b3d into quickwit-oss:main Jun 26, 2024
1 check passed
@idrissneumann
Copy link
Collaborator

Merged @Sh4d1 thanks for the work !

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.

4 participants