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

Allow service.Type="LoadBalancer" #2699

Merged

Conversation

JakeCooper
Copy link
Contributor

Closes #2689 by statically encoding the type to ClusterIP

This allows us to set service.Type="LoadBalancer" such that we can expose the service (and send profiles from non-Kube services)

NOTE(s):

  • This PR has NOT been tested! If someone can point me to an "idiots guide to testing helm packages", rather just some way to import them locally, would be happy to
  • This PR doesn't resolve my other issue of serviceAnnotations (to add the GCP LB annotations to make it an internal LB), but I'll follow that up with a separate PR and issue

@JakeCooper JakeCooper requested a review from a team as a code owner November 16, 2023 03:43
@CLAassistant
Copy link

CLAassistant commented Nov 16, 2023

CLA assistant check
All committers have signed the CLA.

@kolesnikovae
Copy link
Collaborator

Thank you so much for the contribution @JakeCooper!

Could you please sign the CLA? Also, please run make helm/check that will generate templates, and push them to the branch.

This PR has NOT been tested! If someone can point me to an "idiots guide to testing helm packages", rather just some way to import them locally, would be happy to

I'm afraid we don't yet have a fully fledged test suite for our helm chart, therefore the only way to test it is deploying it to the test/dev environment. I'll take care of it :)

@kolesnikovae kolesnikovae self-assigned this Nov 16, 2023
@kolesnikovae kolesnikovae linked an issue Nov 16, 2023 that may be closed by this pull request
@JakeCooper
Copy link
Contributor Author

@kolesnikovae Here's the output but it didn't seem to add anything to the branch?

➜  pyroscope git:(main) make helm/check
GOBIN=/Users/jakecooper/programming/pyroscope/.tmp/bin go install github.com/yannh/kubeconform/cmd/kubeconform@v0.5.0
go: downloading github.com/yannh/kubeconform v0.5.0
go: downloading github.com/xeipuuv/gojsonschema v1.2.0
go: downloading sigs.k8s.io/yaml v1.2.0
go: downloading github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415
go: downloading github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb
GOBIN=/Users/jakecooper/programming/pyroscope/.tmp/bin go install helm.sh/helm/v3/cmd/helm@v3.12.3
go: downloading helm.sh/helm/v3 v3.12.3
go: downloading github.com/moby/term v0.0.0-20221205130635-1aeaba878587
go: downloading github.com/Masterminds/semver/v3 v3.2.1
go: downloading github.com/gofrs/flock v0.8.1
go: downloading github.com/gosuri/uitable v0.0.4
go: downloading k8s.io/apimachinery v0.27.3
go: downloading k8s.io/client-go v0.27.3
go: downloading k8s.io/klog/v2 v2.100.1
go: downloading k8s.io/kubectl v0.27.3
go: downloading github.com/Masterminds/sprig/v3 v3.2.3
go: downloading k8s.io/api v0.27.3
go: downloading k8s.io/cli-runtime v0.27.3
go: downloading github.com/cyphar/filepath-securejoin v0.2.3
go: downloading github.com/mitchellh/copystructure v1.2.0
go: downloading k8s.io/apiextensions-apiserver v0.27.3
go: downloading github.com/evanphx/json-patch v5.6.0+incompatible
go: downloading github.com/Masterminds/vcs v1.13.3
go: downloading github.com/containerd/containerd v1.7.0
go: downloading github.com/sirupsen/logrus v1.9.0
go: downloading oras.land/oras-go v1.2.3
go: downloading github.com/opencontainers/image-spec v1.1.0-rc2.0.20221005185240-3a7f492d3f1b
go: downloading github.com/Masterminds/squirrel v1.5.4
go: downloading github.com/rubenv/sql-migrate v1.3.1
go: downloading github.com/gobwas/glob v0.2.3
go: downloading k8s.io/utils v0.0.0-20230220204549-a5ecb0141aa5
go: downloading sigs.k8s.io/structured-merge-diff/v4 v4.2.3
go: downloading github.com/go-logr/logr v1.2.3
go: downloading github.com/Masterminds/goutils v1.1.1
go: downloading github.com/huandu/xstrings v1.4.0
go: downloading github.com/imdario/mergo v0.3.13
go: downloading github.com/spf13/cast v1.5.0
go: downloading github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de
go: downloading github.com/google/gnostic v0.5.7-v3refs
go: downloading k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f
go: downloading sigs.k8s.io/kustomize/api v0.13.2
go: downloading sigs.k8s.io/kustomize/kyaml v0.14.1
go: downloading github.com/fvbommel/sortorder v1.0.1
go: downloading github.com/mitchellh/reflectwalk v1.0.2
go: downloading k8s.io/component-base v0.27.3
go: downloading github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d
go: downloading github.com/lann/builder v0.0.0-20180802200727-47ae307949d0
go: downloading github.com/docker/cli v23.0.1+incompatible
go: downloading github.com/docker/distribution v2.8.2+incompatible
go: downloading github.com/docker/docker v23.0.1+incompatible
go: downloading github.com/asaskevich/govalidator v0.0.0-20200428143746-21a406dcc535
go: downloading k8s.io/apiserver v0.27.3
go: downloading github.com/mattn/go-runewidth v0.0.9
go: downloading gopkg.in/inf.v0 v0.9.1
go: downloading sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd
go: downloading github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7
go: downloading github.com/peterbourgon/diskv v2.0.1+incompatible
go: downloading github.com/go-gorp/gorp/v3 v3.0.5
go: downloading github.com/chai2010/gettext-go v1.0.2
go: downloading github.com/MakeNowJust/heredoc v1.0.0
go: downloading github.com/mitchellh/go-wordwrap v1.0.0
go: downloading golang.org/x/oauth2 v0.4.0
go: downloading golang.org/x/time v0.0.0-20220210224613-90d013bbcef8
go: downloading github.com/moby/locker v1.0.1
go: downloading github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0
go: downloading github.com/go-openapi/jsonreference v0.20.1
go: downloading github.com/docker/docker-credential-helpers v0.7.0
go: downloading github.com/google/btree v1.0.1
go: downloading github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822
go: downloading go.opentelemetry.io/otel v1.14.0
go: downloading go.opentelemetry.io/otel/trace v1.14.0
go: downloading github.com/go-errors/errors v1.4.2
go: downloading github.com/klauspost/compress v1.16.0
go: downloading github.com/emicklei/go-restful/v3 v3.10.1
go: downloading github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00
go: downloading github.com/xlab/treeprint v1.1.0
go: downloading github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
go: downloading github.com/docker/go-metrics v0.0.1
go: downloading github.com/moby/spdystream v0.2.0
go: downloading go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5
go: downloading github.com/prometheus/common v0.37.0
/Users/jakecooper/programming/pyroscope/.tmp/bin/helm repo add --force-update minio https://charts.min.io/
"minio" has been added to your repositories
/Users/jakecooper/programming/pyroscope/.tmp/bin/helm repo add --force-update grafana https://grafana.github.io/helm-charts
"grafana" has been added to your repositories
/Users/jakecooper/programming/pyroscope/.tmp/bin/helm dependency update ./operations/pyroscope/helm/pyroscope/
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "minio" chart repository
...Successfully got an update from the "victoria-metrics-cluster" chart repository
...Successfully got an update from the "grafana" chart repository
...Successfully got an update from the "signoz" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 2 charts
Downloading grafana-agent from repo https://grafana.github.io/helm-charts
Downloading minio from repo https://charts.min.io/
Deleting outdated charts
/Users/jakecooper/programming/pyroscope/.tmp/bin/helm dependency build ./operations/pyroscope/helm/pyroscope/
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "minio" chart repository
...Successfully got an update from the "signoz" chart repository
...Successfully got an update from the "victoria-metrics-cluster" chart repository
...Successfully got an update from the "grafana" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 2 charts
Downloading grafana-agent from repo https://grafana.github.io/helm-charts
Downloading minio from repo https://charts.min.io/
Deleting outdated charts
mkdir -p ./operations/pyroscope/helm/pyroscope/rendered/
/Users/jakecooper/programming/pyroscope/.tmp/bin/helm template -n default --kube-version "1.22.0" pyroscope-dev ./operations/pyroscope/helm/pyroscope/ \
                | tee ./operations/pyroscope/helm/pyroscope/rendered/single-binary.yaml \
                | /Users/jakecooper/programming/pyroscope/.tmp/bin/kubeconform --summary --strict --kubernetes-version 1.22.0
Summary: 17 resources found parsing stdin - Valid: 17, Invalid: 0, Errors: 0, Skipped: 0
/Users/jakecooper/programming/pyroscope/.tmp/bin/helm template -n default --kube-version "1.22.0" pyroscope-dev ./operations/pyroscope/helm/pyroscope/ --values operations/pyroscope/helm/pyroscope/values-micro-services.yaml \
                | tee ./operations/pyroscope/helm/pyroscope/rendered/micro-services.yaml \
                | /Users/jakecooper/programming/pyroscope/.tmp/bin/kubeconform --summary --strict --kubernetes-version 1.22.0
Summary: 50 resources found parsing stdin - Valid: 50, Invalid: 0, Errors: 0, Skipped: 0
cat operations/pyroscope/helm/pyroscope/values-micro-services.yaml \
                | go run ./tools/yaml-to-json \
                > ./operations/pyroscope/jsonnet/values-micro-services.json
cat operations/pyroscope/helm/pyroscope/values.yaml \
                | go run ./tools/yaml-to-json \
                > ./operations/pyroscope/jsonnet/values.json

Copy link
Collaborator

@kolesnikovae kolesnikovae left a comment

Choose a reason for hiding this comment

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

LGTM

Right now the build is broken, therefore please ignore failed tests (fixed in #2701)

@kolesnikovae kolesnikovae merged commit 0a57f72 into grafana:main Nov 16, 2023
16 of 17 checks passed
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.

Support service.Type="LoadBalancer" in Helm chart Helm-Chart fails with NodePort type
3 participants