Handle v1alpha3 of Compose on Kubernetes API#1615
Conversation
8d99a27 to
899b74e
Compare
Codecov Report
@@ Coverage Diff @@
## master #1615 +/- ##
==========================================
+ Coverage 55.62% 55.84% +0.21%
==========================================
Files 301 302 +1
Lines 20444 20543 +99
==========================================
+ Hits 11372 11472 +100
+ Misses 8253 8245 -8
- Partials 819 826 +7 |
|
@silvin-lubecki PTAL (after #1611, and with #1617) |
silvin-lubecki
left a comment
There was a problem hiding this comment.
Just blocking until #1611 gets merged.
|
@vdemeester can you take a look at it? 🙏 |
Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
899b74e to
592c116
Compare
|
Just rebased. |
silvin-lubecki
left a comment
There was a problem hiding this comment.
LGTM since #1611 is merged.
thaJeztah
left a comment
There was a problem hiding this comment.
Sorry. got dragged away, so didn't complete review yet; these were the nits I found earlier, so let me post those already, and I'll try to revisit the PR later
|
Please sign your commits following these rules: $ git clone -b "handle-v1alpha3" git@github.com:simonferquel/cli.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354512128
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -fAmending updates the existing PR. You DO NOT need to open a new one. |
Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
38a9304 to
2e5981d
Compare
Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>
thaJeztah
left a comment
There was a problem hiding this comment.
Thanks! Ok, so I'm "ok" with this change, but (erm.. well, you know me), left an idea 🤗
If it's out of scope, then "LGTM"
|
|
||
| // NewFactory creates a kubernetes client factory | ||
| func NewFactory(namespace string, config *restclient.Config, clientSet *kubeclient.Clientset) (*Factory, error) { | ||
| func NewFactory(namespace string, config *restclient.Config, clientSet *kubeclient.Clientset, experimental bool) (*Factory, error) { |
There was a problem hiding this comment.
Ok, so looking at this, there's some things I notice;
- This
experimentalonly applies to theStacksAPI; will there be points where we'll have to switch API versions for other clients? - If so; would it be an option to put this
boolon theStacks()function instead? - I don't like booleans; they're confusing, and easily lead to bugs as it's not always clear to see what they do, without looking at the function's signature, and accidentally toggling a boolean is easy.
I guess it's too late for this, but now that we're introducing more functional arguments; we could use them here as well.
Here's a 5-minute quick 'n dirty attempt;
package kubernetes
import (
"github.com/docker/cli/kubernetes"
"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubeclient "k8s.io/client-go/kubernetes"
appsv1beta2 "k8s.io/client-go/kubernetes/typed/apps/v1beta2"
typesappsv1beta2 "k8s.io/client-go/kubernetes/typed/apps/v1beta2"
corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
restclient "k8s.io/client-go/rest"
)
// ClientFactory creates kubernetes clients
type ClientFactory interface {
// ConfigMaps returns a client for kubernetes's config maps
ConfigMaps(ops ...FactoryOp) corev1.ConfigMapInterface
// Secrets returns a client for kubernetes's secrets
Secrets(ops ...FactoryOp) corev1.SecretInterface
// Services returns a client for kubernetes's secrets
Services(ops ...FactoryOp) corev1.ServiceInterface
// Pods returns a client for kubernetes's pods
Pods(ops ...FactoryOp) corev1.PodInterface
// Nodes returns a client for kubernetes's nodes
Nodes(ops ...FactoryOp) corev1.NodeInterface
// ReplicationControllers returns a client for kubernetes replication controllers
ReplicationControllers(ops ...FactoryOp) corev1.ReplicationControllerInterface
// ReplicaSets returns a client for kubernetes replace sets
ReplicaSets(ops ...FactoryOp) typesappsv1beta2.ReplicaSetInterface
// DaemonSets returns a client for kubernetes daemon sets
DaemonSets(ops ...FactoryOp) typesappsv1beta2.DaemonSetInterface
// Stacks returns a client for Docker's Stack on Kubernetes
Stacks(ops ...FactoryOp) (StackClient, error)
}
// Factory is the kubernetes client factory
type Factory struct {
namespace string
config *restclient.Config
coreClientSet corev1.CoreV1Interface
appsClientSet appsv1beta2.AppsV1beta2Interface
clientSet *kubeclient.Clientset
stackAPIVersion *kubernetes.StackVersion
}
// NewFactory creates a kubernetes client factory
func NewFactory(ops ...FactoryOp) (*Factory, error) {
f := &Factory{}
ops = append(ops, WithDefaultStackAPI)
for _, o := range ops {
if err := o(f); err != nil {
return nil, err
}
}
coreClientSet, err := corev1.NewForConfig(f.config)
if err != nil {
return nil, err
}
appsClientSet, err := appsv1beta2.NewForConfig(f.config)
if err != nil {
return nil, err
}
f.coreClientSet = coreClientSet
f.appsClientSet = appsClientSet
return f, nil
}
// ConfigMaps returns a client for kubernetes's config maps
func (s *Factory) ConfigMaps(ops ...FactoryOp) corev1.ConfigMapInterface {
return s.coreClientSet.ConfigMaps(s.namespace)
}
// Secrets returns a client for kubernetes's secrets
func (s *Factory) Secrets(ops ...FactoryOp) corev1.SecretInterface {
return s.coreClientSet.Secrets(s.namespace)
}
// Services returns a client for kubernetes's secrets
func (s *Factory) Services(ops ...FactoryOp) corev1.ServiceInterface {
return s.coreClientSet.Services(s.namespace)
}
// Pods returns a client for kubernetes's pods
func (s *Factory) Pods(ops ...FactoryOp) corev1.PodInterface {
return s.coreClientSet.Pods(s.namespace)
}
// Nodes returns a client for kubernetes's nodes
func (s *Factory) Nodes(ops ...FactoryOp) corev1.NodeInterface {
return s.coreClientSet.Nodes()
}
// ReplicationControllers returns a client for kubernetes replication controllers
func (s *Factory) ReplicationControllers(ops ...FactoryOp) corev1.ReplicationControllerInterface {
return s.coreClientSet.ReplicationControllers(s.namespace)
}
// ReplicaSets returns a client for kubernetes replace sets
func (s *Factory) ReplicaSets(ops ...FactoryOp) typesappsv1beta2.ReplicaSetInterface {
return s.appsClientSet.ReplicaSets(s.namespace)
}
// DaemonSets returns a client for kubernetes daemon sets
func (s *Factory) DaemonSets(ops ...FactoryOp) typesappsv1beta2.DaemonSetInterface {
return s.appsClientSet.DaemonSets(s.namespace)
}
// Stacks returns a client for Docker's Stack on Kubernetes
func (s *Factory) Stacks(ops ...FactoryOp) (StackClient, error) {
f := s
ops = append(ops, WithDefaultStackAPI)
for _, o := range ops {
if err := o(f); err != nil {
return nil, err
}
}
switch *s.stackAPIVersion {
case kubernetes.StackAPIV1Beta1:
return newStackV1Beta1(s.config, s.namespace)
case kubernetes.StackAPIV1Beta2:
return newStackV1Beta2(s.config, s.namespace)
case kubernetes.StackAPIV1Alpha3:
return newStackV1Alpha3(s.config, s.namespace)
default:
return nil, errors.Errorf("unsupported stack API version: %q", s.stackAPIVersion)
}
}
type FactoryOp func(f *Factory) error
func WithNameSpace(namespace string) FactoryOp {
return func(f *Factory) error {
f.namespace = namespace
return nil
}
}
func WithAllNamespaces(f *Factory) error {
f.namespace = metav1.NamespaceAll
return nil
}
func WithClientSet(clientSet *kubeclient.Clientset) FactoryOp {
return func(f *Factory) error {
f.clientSet = clientSet
return nil
}
}
func WithConfig(config *restclient.Config) FactoryOp {
return func(f *Factory) error {
f.config = config
return nil
}
}
func WithExperimentalStackAPI(f *Factory) error {
version, err := kubernetes.GetStackAPIVersion(f.clientSet.Discovery(), true)
if err != nil {
return err
}
f.stackAPIVersion = &version
return nil
}
func WithDefaultStackAPI(f *Factory) error {
if f.stackAPIVersion != nil {
return nil
}
version, err := kubernetes.GetStackAPIVersion(f.clientSet.Discovery(), false)
if err != nil {
return err
}
f.stackAPIVersion = &version
return nil
}diff (in case it's still an option, and if it helps you);
Details
diff --git a/cli/command/stack/kubernetes/client.go b/cli/command/stack/kubernetes/client.go
index fd516bb5..c94fe592 100644
--- a/cli/command/stack/kubernetes/client.go
+++ b/cli/command/stack/kubernetes/client.go
@@ -11,97 +11,181 @@ import (
restclient "k8s.io/client-go/rest"
)
+// ClientFactory creates kubernetes clients
+type ClientFactory interface {
+ // ConfigMaps returns a client for kubernetes's config maps
+ ConfigMaps(ops ...FactoryOp) corev1.ConfigMapInterface
+
+ // Secrets returns a client for kubernetes's secrets
+ Secrets(ops ...FactoryOp) corev1.SecretInterface
+
+ // Services returns a client for kubernetes's secrets
+ Services(ops ...FactoryOp) corev1.ServiceInterface
+
+ // Pods returns a client for kubernetes's pods
+ Pods(ops ...FactoryOp) corev1.PodInterface
+
+ // Nodes returns a client for kubernetes's nodes
+ Nodes(ops ...FactoryOp) corev1.NodeInterface
+
+ // ReplicationControllers returns a client for kubernetes replication controllers
+ ReplicationControllers(ops ...FactoryOp) corev1.ReplicationControllerInterface
+
+ // ReplicaSets returns a client for kubernetes replace sets
+ ReplicaSets(ops ...FactoryOp) typesappsv1beta2.ReplicaSetInterface
+
+ // DaemonSets returns a client for kubernetes daemon sets
+ DaemonSets(ops ...FactoryOp) typesappsv1beta2.DaemonSetInterface
+
+ // Stacks returns a client for Docker's Stack on Kubernetes
+ Stacks(ops ...FactoryOp) (StackClient, error)
+}
+
// Factory is the kubernetes client factory
type Factory struct {
- namespace string
- config *restclient.Config
- coreClientSet corev1.CoreV1Interface
- appsClientSet appsv1beta2.AppsV1beta2Interface
- clientSet *kubeclient.Clientset
- experimental bool
+ namespace string
+ config *restclient.Config
+ coreClientSet corev1.CoreV1Interface
+ appsClientSet appsv1beta2.AppsV1beta2Interface
+ clientSet *kubeclient.Clientset
+ stackAPIVersion *kubernetes.StackVersion
}
// NewFactory creates a kubernetes client factory
-func NewFactory(namespace string, config *restclient.Config, clientSet *kubeclient.Clientset, experimental bool) (*Factory, error) {
- coreClientSet, err := corev1.NewForConfig(config)
+func NewFactory(ops ...FactoryOp) (*Factory, error) {
+ f := &Factory{}
+
+ ops = append(ops, WithDefaultStackAPI)
+ for _, o := range ops {
+ if err := o(f); err != nil {
+ return nil, err
+ }
+ }
+
+ coreClientSet, err := corev1.NewForConfig(f.config)
if err != nil {
return nil, err
}
- appsClientSet, err := appsv1beta2.NewForConfig(config)
+ appsClientSet, err := appsv1beta2.NewForConfig(f.config)
if err != nil {
return nil, err
}
- return &Factory{
- namespace: namespace,
- config: config,
- coreClientSet: coreClientSet,
- appsClientSet: appsClientSet,
- clientSet: clientSet,
- experimental: experimental,
- }, nil
+ f.coreClientSet = coreClientSet
+ f.appsClientSet = appsClientSet
+
+ return f, nil
}
// ConfigMaps returns a client for kubernetes's config maps
-func (s *Factory) ConfigMaps() corev1.ConfigMapInterface {
+func (s *Factory) ConfigMaps(ops ...FactoryOp) corev1.ConfigMapInterface {
return s.coreClientSet.ConfigMaps(s.namespace)
}
// Secrets returns a client for kubernetes's secrets
-func (s *Factory) Secrets() corev1.SecretInterface {
+func (s *Factory) Secrets(ops ...FactoryOp) corev1.SecretInterface {
return s.coreClientSet.Secrets(s.namespace)
}
// Services returns a client for kubernetes's secrets
-func (s *Factory) Services() corev1.ServiceInterface {
+func (s *Factory) Services(ops ...FactoryOp) corev1.ServiceInterface {
return s.coreClientSet.Services(s.namespace)
}
// Pods returns a client for kubernetes's pods
-func (s *Factory) Pods() corev1.PodInterface {
+func (s *Factory) Pods(ops ...FactoryOp) corev1.PodInterface {
return s.coreClientSet.Pods(s.namespace)
}
// Nodes returns a client for kubernetes's nodes
-func (s *Factory) Nodes() corev1.NodeInterface {
+func (s *Factory) Nodes(ops ...FactoryOp) corev1.NodeInterface {
return s.coreClientSet.Nodes()
}
// ReplicationControllers returns a client for kubernetes replication controllers
-func (s *Factory) ReplicationControllers() corev1.ReplicationControllerInterface {
+func (s *Factory) ReplicationControllers(ops ...FactoryOp) corev1.ReplicationControllerInterface {
return s.coreClientSet.ReplicationControllers(s.namespace)
}
// ReplicaSets returns a client for kubernetes replace sets
-func (s *Factory) ReplicaSets() typesappsv1beta2.ReplicaSetInterface {
+func (s *Factory) ReplicaSets(ops ...FactoryOp) typesappsv1beta2.ReplicaSetInterface {
return s.appsClientSet.ReplicaSets(s.namespace)
}
// DaemonSets returns a client for kubernetes daemon sets
-func (s *Factory) DaemonSets() typesappsv1beta2.DaemonSetInterface {
+func (s *Factory) DaemonSets(ops ...FactoryOp) typesappsv1beta2.DaemonSetInterface {
return s.appsClientSet.DaemonSets(s.namespace)
}
// Stacks returns a client for Docker's Stack on Kubernetes
-func (s *Factory) Stacks(allNamespaces bool) (StackClient, error) {
- version, err := kubernetes.GetStackAPIVersion(s.clientSet.Discovery(), s.experimental)
- if err != nil {
- return nil, err
- }
- namespace := s.namespace
- if allNamespaces {
- namespace = metav1.NamespaceAll
+func (s *Factory) Stacks(ops ...FactoryOp) (StackClient, error) {
+ f := s
+
+ ops = append(ops, WithDefaultStackAPI)
+ for _, o := range ops {
+ if err := o(f); err != nil {
+ return nil, err
+ }
}
- switch version {
+ switch *s.stackAPIVersion {
case kubernetes.StackAPIV1Beta1:
- return newStackV1Beta1(s.config, namespace)
+ return newStackV1Beta1(s.config, s.namespace)
case kubernetes.StackAPIV1Beta2:
- return newStackV1Beta2(s.config, namespace)
+ return newStackV1Beta2(s.config, s.namespace)
case kubernetes.StackAPIV1Alpha3:
- return newStackV1Alpha3(s.config, namespace)
+ return newStackV1Alpha3(s.config, s.namespace)
default:
- return nil, errors.Errorf("unsupported stack API version: %q", version)
+ return nil, errors.Errorf("unsupported stack API version: %q", s.stackAPIVersion)
+ }
+}
+
+type FactoryOp func(f *Factory) error
+
+func WithNameSpace(namespace string) FactoryOp {
+ return func(f *Factory) error {
+ f.namespace = namespace
+ return nil
+ }
+}
+
+func WithAllNamespaces(f *Factory) error {
+ f.namespace = metav1.NamespaceAll
+ return nil
+}
+
+func WithClientSet(clientSet *kubeclient.Clientset) FactoryOp {
+ return func(f *Factory) error {
+ f.clientSet = clientSet
+ return nil
+ }
+}
+
+func WithConfig(config *restclient.Config) FactoryOp {
+ return func(f *Factory) error {
+ f.config = config
+ return nil
+ }
+}
+
+func WithExperimentalStackAPI(f *Factory) error {
+ version, err := kubernetes.GetStackAPIVersion(f.clientSet.Discovery(), true)
+ if err != nil {
+ return err
+ }
+ f.stackAPIVersion = &version
+ return nil
+}
+
+func WithDefaultStackAPI(f *Factory) error {
+ if f.stackAPIVersion != nil {
+ return nil
+ }
+ version, err := kubernetes.GetStackAPIVersion(f.clientSet.Discovery(), false)
+ if err != nil {
+ return err
}
+ f.stackAPIVersion = &version
+ return nil
}There was a problem hiding this comment.
I think we are using a Jackhammer to put a pin in a butter brick here :).
Without kidding, the responsibility of the factory is to do version negociation with the server to decide which implementation of the stackClient abstraction we want to use. Negociation was previously parameter-less, and now has a single parameter influencing the decision. I don't think this will change anytime soon (pick the highest common version, including or excluding experimental ones), and so I don't thing adding options there makes sense. Plus your example is error prone, and potentially has side effects on the factory fields each time you call a factory method.
There was a problem hiding this comment.
@thaJeztah WDYT ? should we merge as is ?
Based on #1611Compose on Kubernetes features under active development are introduced in API version v1alpha3, which is a superset of v1beta2, that will never break v1beta2 valid constructs, but that we consider as unstable (and breakable) until we promote it to v1beta3.
To expose those "in-development features", we need the CLI to be able to handle this API version, under the
experimentalflag. This PR handles that.- What I did
- How I did it
- How to verify it
- A picture of a cute animal (not mandatory but encouraged)
