Skip to content

Commit 5b324eb

Browse files
authored
Improve Install UX (#17)
We can use additional printer columns in the CRD to report configuration status is a more user friendly way than trawling though description YAML. Also noticed that the error strings are somewhat clunky and abigouous so have cleaned these up too.
1 parent 80d180b commit 5b324eb

File tree

4 files changed

+25
-11
lines changed

4 files changed

+25
-11
lines changed

crds/servicebroker.couchbase.com_servicebrokerconfigs.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,22 @@ metadata:
88
creationTimestamp: null
99
name: servicebrokerconfigs.servicebroker.couchbase.com
1010
spec:
11+
additionalPrinterColumns:
12+
- JSONPath: .status.conditions[?(@.type=="ConfigurationValid")].status
13+
description: whether the configuration is valid
14+
name: valid
15+
type: string
16+
- JSONPath: .metadata.creationTimestamp
17+
name: age
18+
type: date
1119
group: servicebroker.couchbase.com
1220
names:
1321
kind: ServiceBrokerConfig
1422
listKind: ServiceBrokerConfigList
1523
plural: servicebrokerconfigs
1624
singular: servicebrokerconfig
1725
scope: Namespaced
26+
subresources: {}
1827
validation:
1928
openAPIV3Schema:
2029
properties:

documentation/modules/ROOT/pages/install/kubernetes.adoc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,22 +104,25 @@ deployment.extensions/couchbase-service-broker condition met
104104
====
105105
The Service Broker does not use a dynamic admission controller to validate the configuration at present.
106106
It does, however, perform validation internally and report this back to the user through a status in the configuration resource.
107-
If for any reason the deployment does not become ready, first check the logs for information:
107+
If you have made a configuration error, the Service Broker `Deployment` will not become ready.
108+
You can see the validation status directly through the CLI:
108109
109110
[source,console]
110111
----
111-
$ kubectl logs -f deployment/couchbase-service-broker
112+
$ kubectl get servicebrokerconfigs
113+
NAME VALID AGE
114+
couchbase-service-broker True 9m
112115
----
113116
114-
If the logs indicate you have a configuration error you can examine this in more detail with the following command:
117+
If you have a configuration error, you can examine this in more detail with the following command:
115118
116119
[source,console]
117120
----
118121
$ kubectl describe servicebrokerconfigs
119122
Status:
120123
Conditions:
121124
Last Transition Time: 2020-04-06T15:51:17Z
122-
Message: template couchbase-operator-rolebinding referenced by configuration couchbase-developer-private service instance must exist
125+
Message: template 'couchbase-operator-rolebinding', referenced by binding 'couchbase-developer-private' service instance, must exist
123126
Reason: ValidationFailed
124127
Status: False
125128
Type: ConfigurationValid

pkg/apis/servicebroker/v1alpha1/types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ const (
3434
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
3535
// +kubebuilder:resource:categories=all;couchbase
3636
// +kubebuilder:resource:scope=Namespaced
37+
// +kubebuilder:printcolumn:name="valid",type="string",JSONPath=".status.conditions[?(@.type==\"ConfigurationValid\")].status",description="whether the configuration is valid"
38+
// +kubebuilder:printcolumn:name="age",type="date",JSONPath=".metadata.creationTimestamp"
3739
type ServiceBrokerConfig struct {
3840
metav1.TypeMeta `json:",inline"`
3941
metav1.ObjectMeta `json:"metadata,omitempty"`

pkg/config/validation.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func validate(config *v1.ServiceBrokerConfig) error {
5151
// Each service plan must have a service binding.
5252
binding := getBindingForServicePlan(config, service.Name, plan.Name)
5353
if binding == nil {
54-
return fmt.Errorf("service plan %s for offering %s does not have a configuration binding", plan.Name, service.Name)
54+
return fmt.Errorf("service plan '%s' for offering '%s' does not have a binding", plan.Name, service.Name)
5555
}
5656

5757
// Only bindable service plans may have templates for bindings.
@@ -61,11 +61,11 @@ func validate(config *v1.ServiceBrokerConfig) error {
6161
}
6262

6363
if !bindable && binding.ServiceBinding != nil {
64-
return fmt.Errorf("service plan %s for offering %s not bindable, but configuration binding %s defines service binding configuarion", plan.Name, service.Name, binding.Name)
64+
return fmt.Errorf("service plan '%s' for offering '%s' not bindable, but binding '%s' defines service binding configuarion", plan.Name, service.Name, binding.Name)
6565
}
6666

6767
if bindable && binding.ServiceBinding == nil {
68-
return fmt.Errorf("service plan %s for offering %s bindable, but configuration binding %s does not define service binding configuarion", plan.Name, service.Name, binding.Name)
68+
return fmt.Errorf("service plan '%s' for offering '%s' bindable, but binding '%s' does not define service binding configuarion", plan.Name, service.Name, binding.Name)
6969
}
7070
}
7171
}
@@ -74,26 +74,26 @@ func validate(config *v1.ServiceBrokerConfig) error {
7474
for _, binding := range config.Spec.Bindings {
7575
// Bindings cannot do nothing.
7676
if len(binding.ServiceInstance.Registry) == 0 && len(binding.ServiceInstance.Templates) == 0 {
77-
return fmt.Errorf("configuration binding %s does nothing for service instances", binding.Name)
77+
return fmt.Errorf("binding '%s' does nothing for service instances", binding.Name)
7878
}
7979

8080
if binding.ServiceBinding != nil {
8181
if len(binding.ServiceBinding.Registry) == 0 && len(binding.ServiceBinding.Templates) == 0 {
82-
return fmt.Errorf("configuration binding %s does nothing for service bindings", binding.Name)
82+
return fmt.Errorf("binding '%s' does nothing for service bindings", binding.Name)
8383
}
8484
}
8585

8686
// Binding templates must exist.
8787
for _, template := range binding.ServiceInstance.Templates {
8888
if getTemplateByName(config, template) == nil {
89-
return fmt.Errorf("template %s referenced by configuration %s service instance must exist", template, binding.Name)
89+
return fmt.Errorf("template '%s', referenced by binding '%s' service instance, must exist", template, binding.Name)
9090
}
9191
}
9292

9393
if binding.ServiceBinding != nil {
9494
for _, template := range binding.ServiceBinding.Templates {
9595
if getTemplateByName(config, template) == nil {
96-
return fmt.Errorf("template %s referenced by configuration %s service binding must exist", template, binding.Name)
96+
return fmt.Errorf("template '%s', referenced by binding '%s' service binding, must exist", template, binding.Name)
9797
}
9898
}
9999
}

0 commit comments

Comments
 (0)