-
Notifications
You must be signed in to change notification settings - Fork 79
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
Multigroup structure #187
Multigroup structure #187
Conversation
path: github.com/k8ssandra/k8ssandra-operator/api/v1alpha1 | ||
version: v1alpha1 | ||
path: github.com/k8ssandra/k8ssandra-operator/apis/config/v1beta1 | ||
version: v1beta1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change the version? I assume because of the path changes. I ask though because I think it is understood that alpha APIs can have breaking changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This namespace (config.k8ssandra.io/v1beta1) is used by cass-operator. It shouldn't be different in k8ssandra-operator.
PROJECT
Outdated
@@ -13,29 +14,31 @@ resources: | |||
controller: true | |||
group: k8ssandra.io | |||
kind: K8ssandraCluster | |||
path: github.com/k8ssandra/k8ssandra-operator/api/v1alpha1 | |||
path: github.com/k8ssandra/k8ssandra-operator/apis/core/v1alpha1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of a minor nit. What qualifies K8ssandraCluster as core and others as not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming. I thought apis/cluster would have been somewhat confusing given we have so many clusters already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that cluster would be confusing. I think k8ssandracluster is too long. Not sure about just k8ssandra.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apis/k8ssandra/ ? Sounds fine as well. It's apis/cassandra/ in cass-operator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then let's go with apis/k8ssandra
.
@burmanm I had merged another PR ahead of this which caused conflicts. Let me know when you rebase and resolve the conflicts and we'll get this merged. |
Done. |
@burmanm I submitted a PR against your branch with a fix for the api server timeouts. |
Move controllers to multigroup struct, refactor some testing Fix create-clientconfig Rename apis/core to apis/k8ssandra
Need to cancel the context. It is discussed in kubernetes-sigs/controller-runtime#1571 (comment).
What this PR does:
Modifies the structure of apis and controllers to a multigroup one (kubebuilder v3).
Which issue(s) this PR fixes:
Fixes #93
Checklist