operator-framework / operator-sdk

SDK for building Kubernetes applications. Provides high level APIs, useful abstractions, and project scaffolding.
https://sdk.operatorframework.io
Apache License 2.0
7.1k stars 1.73k forks source link

feat: support helm dryrun=server #6691

Closed holyspectral closed 3 months ago

holyspectral commented 4 months ago

Description of the change:

This is to fix https://github.com/operator-framework/operator-sdk/issues/5728. In this PR, new helm option, --dry-run=server is introduced, so lookup() can be used with helm operator.

This feature is disabled by default to keep backward compatibility. To enable this feature, specify this in watches.yaml

    dryrunOption: server

Motivation for the change:

lookup() in helm is a very useful command. However, it's not supported by helm xxx --dry-run and helm template before 3.13. This causes issues in some tools, e.g., argocd and helm operator. This could cause repetitive reconcile or frequent certificate changes depending on how lookup() is used.

Starting in Helm 3.13, helm provides a new option dryrun=server (https://github.com/helm/helm/pull/9426), which enabled users to contact API server when running helm. This way, lookup() can get result from API server and work as it's intended to be.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

holyspectral commented 3 months ago

Fixed the CamelCase and added a simple test case.

holyspectral commented 3 months ago

Hi @everettraven I rebased to master branch and I can reproduce the error on my local. As your recommendation, I added these files in my latest commit and it seems to fix the issue. However, I'm not entirely sure about changing .cncf-maintainers.

.cncf-maintainers
testdata/go/v4/memcached-operator/Makefile
testdata/go/v4/monitoring/memcached-operator/Makefile
testdata/helm/memcached-operator/Makefile

Could you help to double confirm if it's okay? Thanks!

everettraven commented 3 months ago

Hi @everettraven I rebased to master branch and I can reproduce the error on my local. As your recommendation, I added these files in my latest commit and it seems to fix the issue. However, I'm not entirely sure about changing .cncf-maintainers.

.cncf-maintainers
testdata/go/v4/memcached-operator/Makefile
testdata/go/v4/monitoring/memcached-operator/Makefile
testdata/helm/memcached-operator/Makefile

Could you help to double confirm if it's okay? Thanks!

@holyspectral Looks like that .cncf-maintainers change should have been made as part of https://github.com/operator-framework/operator-sdk/pull/6711 but was missed and generated as part of your PR. Should be OK

everettraven commented 3 months ago

According to the DCO check one of the commits is missing a sign-off: https://github.com/operator-framework/operator-sdk/pull/6691/checks?check_run_id=23201517452

holyspectral commented 3 months ago

Thanks. The DCO error has been fixed now.

holyspectral commented 3 months ago

Looks like there is still some error due to docs:

Checking 784 external links...
Ran on 171 files!

- /target/docs/best-practices/pod-security-standards/index.html
  *  External link https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/v3/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml failed: 404 No error
- /target/docs/building-operators/golang/tutorial/index.html
  *  External link https://github.com/operator-framework/operator-sdk/blob/latest/testdata/go/v3/memcached-operator/controllers/memcached_controller.go failed: 404 No error
- /target/docs/upgrading-sdk-version/v1.18.0/index.html
  *  External link https://github.com/operator-framework/operator-sdk/commit/324ca13a994c7d7749708d550c0ea60d1df4cd0d failed: response code 0 means something's wrong.
             It's possible libcurl couldn't connect to the server or perhaps the request timed out.
             Sometimes, making too many requests at once also breaks things.
             Either way, the return message (if any) from the server is: Failed sending data to the peer

These are potentially related to https://github.com/operator-framework/operator-sdk/pull/6664 since testdata/go/v3 is removed.

holyspectral commented 3 months ago

v3/memcached-operator are all changed to v4/memcached-operator now. My internet connection is not good enough to complete make test-docs in my local, but at least no 404 anymore.

@everettraven can you help to take a look and rerun the test? Thanks!

hkraal commented 1 month ago

@everettraven It seems that changes from this PR aren't reflected at https://sdk.operatorframework.io/docs/building-operators/golang/tutorial/ as the memcached_controller link is still pointing to v3.

Is there any chance to update the docs?