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

Updating controller-runtime and dependencies #740

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

burmanm
Copy link
Contributor

@burmanm burmanm commented Dec 19, 2024

What this PR does:

Which issue(s) this PR fixes:
Fixes #718
Fixes #527
Fixes #617

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CHANGELOG.md updated (not required for documentation PRs)
  • CLA Signed: DataStax CLA

…ons. Remove the use of operatorConfig (since it was removed from controller-runtime) and replace the controller-runtime parameters as cmd line options. Remove kube_rbac module for metrics authentication as it was deprecated also, instead replace it with the option of using secured metrics.

Move serverconfig related stuff out from the apis and move them to the serverconfig pkg. This is part of the cleanup of external dependencies from apis package.
…move cert-manager annotation injections to webhook component, fix some Kustomize 5.x deprecations
… cluster deployment the default for "make deploy"
…r structure in kubebuilder and make it its own struct and not part of CassandraDatacenter object. This removes controller-runtime dependency from the apis.
…e replacements will not work correctly for Certificates
@burmanm burmanm marked this pull request as ready for review December 20, 2024 13:58
@burmanm burmanm requested a review from a team as a code owner December 20, 2024 13:58
Comment on lines +2 to +4
- op: add
path: /spec/template/spec/containers/0/args/0
value: --metrics-bind-address=:8443
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: I assume we'll need to do this in the helm charts as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the Helm chart will require changes as well.

@@ -1,5 +1,5 @@
/*
Copyright 2021.
Copyright 2024 Just Me.
Copy link
Contributor

Choose a reason for hiding this comment

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

🙂

// Note that you must have the required binaries setup under the bin directory to perform
// the tests directly. When we run make test it will be setup and used automatically.
BinaryAssetsDirectory: filepath.Join("..", "..", "..", "bin", "k8s",
fmt.Sprintf("1.31.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: could we make that version a variable? I'm worried that this is quite hidden in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I can put it to the start of the file, but this isn't used for normal envtest runs - only if running from vscode directly.

@@ -1,12 +1,37 @@
---
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: why is a mutating webhook introduced?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, actually we always had the feature, but if you tried to use it, it wouldn't work. This is for the DefaultingWebhook that was always present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants