From 22d5e4810b5d621f0c9bd6c02a74ac90d04370c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mika=C3=ABl=20Cluseau?= Date: Wed, 23 Aug 2017 13:44:45 +1100 Subject: [PATCH] refactor(flexvolume): simplify capabilities handling --- pkg/volume/flexvolume/BUILD | 1 + pkg/volume/flexvolume/driver-call.go | 46 ++++++++--------------- pkg/volume/flexvolume/driver-call_test.go | 32 ++++++++++++++++ pkg/volume/flexvolume/mounter-defaults.go | 2 +- pkg/volume/flexvolume/plugin.go | 13 +++---- 5 files changed, 55 insertions(+), 39 deletions(-) create mode 100644 pkg/volume/flexvolume/driver-call_test.go diff --git a/pkg/volume/flexvolume/BUILD b/pkg/volume/flexvolume/BUILD index 3a78103ab86..b590ed98acf 100644 --- a/pkg/volume/flexvolume/BUILD +++ b/pkg/volume/flexvolume/BUILD @@ -42,6 +42,7 @@ go_test( "attacher_test.go", "common_test.go", "detacher_test.go", + "driver-call_test.go", "flexvolume_test.go", "mounter_test.go", "plugin_test.go", diff --git a/pkg/volume/flexvolume/driver-call.go b/pkg/volume/flexvolume/driver-call.go index aad05eeb159..e3e4527ddc9 100644 --- a/pkg/volume/flexvolume/driver-call.go +++ b/pkg/volume/flexvolume/driver-call.go @@ -58,9 +58,6 @@ const ( optionKeyPodUID = "kubernetes.io/pod.uid" optionKeyServiceAccountName = "kubernetes.io/serviceAccount.name" - - attachCapability = "attach" - selinuxRelabelCapability = "selinuxRelabel" ) const ( @@ -83,11 +80,6 @@ type DriverCall struct { args []string } -type driverCapabilities struct { - attach bool - selinuxRelabel bool -} - func (plugin *flexVolumePlugin) NewDriverCall(command string) *DriverCall { return plugin.NewDriverCallWithTimeout(command, 0) } @@ -210,7 +202,19 @@ type DriverStatus struct { // Returns capabilities of the driver. // By default we assume all the capabilities are supported. // If the plugin does not support a capability, it can return false for that capability. - Capabilities map[string]bool + Capabilities *DriverCapabilities `json:",omitempty"` +} + +type DriverCapabilities struct { + Attach bool `json:"attach"` + SELinuxRelabel bool `json:"selinuxRelabel"` +} + +func defaultCapabilities() *DriverCapabilities { + return &DriverCapabilities{ + Attach: true, + SELinuxRelabel: true, + } } // isCmdNotSupportedErr checks if the error corresponds to command not supported by @@ -226,7 +230,9 @@ func isCmdNotSupportedErr(err error) bool { // handleCmdResponse processes the command output and returns the appropriate // error code or message. func handleCmdResponse(cmd string, output []byte) (*DriverStatus, error) { - var status DriverStatus + status := DriverStatus{ + Capabilities: defaultCapabilities(), + } if err := json.Unmarshal(output, &status); err != nil { glog.Errorf("Failed to unmarshal output for command: %s, output: %q, error: %s", cmd, string(output), err.Error()) return nil, err @@ -241,23 +247,3 @@ func handleCmdResponse(cmd string, output []byte) (*DriverStatus, error) { return &status, nil } - -// getDriverCapabilities returns the reported capabilities as returned by driver's init() function -func (ds *DriverStatus) getDriverCapabilities() *driverCapabilities { - driverCaps := &driverCapabilities{ - attach: true, - selinuxRelabel: true, - } - - // Check if driver supports SELinux Relabeling of mounted volume - if dcap, ok := ds.Capabilities[selinuxRelabelCapability]; ok { - driverCaps.selinuxRelabel = dcap - } - - // Check whether the plugin is attachable. - if dcap, ok := ds.Capabilities[attachCapability]; ok { - driverCaps.attach = dcap - } - - return driverCaps -} diff --git a/pkg/volume/flexvolume/driver-call_test.go b/pkg/volume/flexvolume/driver-call_test.go new file mode 100644 index 00000000000..77f71f7b8f8 --- /dev/null +++ b/pkg/volume/flexvolume/driver-call_test.go @@ -0,0 +1,32 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package flexvolume + +import ( + "testing" +) + +func TestHandleResponseDefaults(t *testing.T) { + ds, err := handleCmdResponse("test", []byte(`{"status": "Success"}`)) + if err != nil { + t.Error("error: ", err) + } + + if *ds.Capabilities != *defaultCapabilities() { + t.Error("wrong default capabilities: ", *ds.Capabilities) + } +} diff --git a/pkg/volume/flexvolume/mounter-defaults.go b/pkg/volume/flexvolume/mounter-defaults.go index a3996d4da3c..b3cd9430ff5 100644 --- a/pkg/volume/flexvolume/mounter-defaults.go +++ b/pkg/volume/flexvolume/mounter-defaults.go @@ -47,7 +47,7 @@ func (f *mounterDefaults) GetAttributes() volume.Attributes { return volume.Attributes{ ReadOnly: f.readOnly, Managed: !f.readOnly, - SupportsSELinux: f.flexVolume.plugin.capabilities.selinuxRelabel, + SupportsSELinux: f.flexVolume.plugin.capabilities.SELinuxRelabel, } } diff --git a/pkg/volume/flexvolume/plugin.go b/pkg/volume/flexvolume/plugin.go index 2e54aae964c..942be15f653 100644 --- a/pkg/volume/flexvolume/plugin.go +++ b/pkg/volume/flexvolume/plugin.go @@ -42,8 +42,8 @@ type flexVolumePlugin struct { runner exec.Interface sync.Mutex - capabilities *driverCapabilities unsupportedCommands []string + capabilities DriverCapabilities } type flexVolumeAttachablePlugin struct { @@ -65,19 +65,16 @@ func NewFlexVolumePlugin(pluginDir, name string) (volume.VolumePlugin, error) { unsupportedCommands: []string{}, } - // Retrieve driver reported capabilities + // Initialize the plugin and probe the capabilities call := flexPlugin.NewDriverCall(initCmd) ds, err := call.Run() if err != nil { return nil, err } + flexPlugin.capabilities = *ds.Capabilities - driverCaps := ds.getDriverCapabilities() - flexPlugin.capabilities = driverCaps - - // Check whether the plugin is attachable. - if driverCaps.attach { - // Plugin supports attach/detach by default, so return flexVolumeAttachablePlugin + if flexPlugin.capabilities.Attach { + // Plugin supports attach/detach, so return flexVolumeAttachablePlugin return &flexVolumeAttachablePlugin{flexVolumePlugin: flexPlugin}, nil } else { return flexPlugin, nil