From 63ee9b3f4f9b1286b2f4a6813cdeb767661a1a4c Mon Sep 17 00:00:00 2001 From: Michelle Au Date: Thu, 2 Apr 2020 13:47:56 -0700 Subject: [PATCH] Clean up event messages for errors. Change-Id: Ib70b50e676b917c4d976f32ee7a19f8fc63b6bc6 --- pkg/volume/glusterfs/glusterfs.go | 30 +++++++---- pkg/volume/quobyte/quobyte.go | 13 ++++- pkg/volume/scaleio/sio_client.go | 71 ++++++++++++++++++-------- pkg/volume/storageos/storageos_util.go | 12 +++-- 4 files changed, 91 insertions(+), 35 deletions(-) diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index 8419039d265..698c4e895f0 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -670,8 +670,9 @@ func (d *glusterfsVolumeDeleter) Delete() error { err = cli.VolumeDelete(volumeID) if err != nil { if dstrings.TrimSpace(err.Error()) != errIDNotFound { - klog.Errorf("failed to delete volume %s: %v", volumeName, err) - return fmt.Errorf("failed to delete volume %s: %v", volumeName, err) + // don't log error details from client calls in events + klog.V(4).Infof("failed to delete volume %s: %v", volumeName, err) + return fmt.Errorf("failed to delete volume: see kube-controller-manager.log for details") } klog.V(2).Infof("volume %s not present in heketi, ignoring", volumeName) } @@ -816,7 +817,9 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi volumeReq := &gapi.VolumeCreateRequest{Size: sz, Name: customVolumeName, Clusters: clusterIDs, Gid: gid64, Durability: p.volumeType, GlusterVolumeOptions: p.volumeOptions, Snapshot: snaps} volume, err := cli.VolumeCreate(volumeReq) if err != nil { - return nil, 0, "", fmt.Errorf("failed to create volume: %v", err) + // don't log error details from client calls in events + klog.V(4).Infof("failed to create volume: %v", err) + return nil, 0, "", fmt.Errorf("failed to create volume: see kube-controller-manager.log for details") } klog.V(1).Infof("volume with size %d and name %s created", volume.Size, volume.Name) volID = volume.Id @@ -837,7 +840,8 @@ func (p *glusterfsVolumeProvisioner) CreateVolume(gid int) (r *v1.GlusterfsPersi if err != nil { deleteErr := cli.VolumeDelete(volume.Id) if deleteErr != nil { - klog.Errorf("failed to delete volume: %v, manual deletion of the volume required", deleteErr) + // don't log error details from client calls in events + klog.V(4).Infof("failed to delete volume: %v, manual deletion of the volume required", deleteErr) } klog.V(3).Infof("failed to update endpoint, deleting %s", endpoint) err = kubeClient.CoreV1().Services(epNamespace).Delete(context.TODO(), epServiceName, metav1.DeleteOptions{}) @@ -955,7 +959,9 @@ func parseSecret(namespace, secretName string, kubeClient clientset.Interface) ( func getClusterNodes(cli *gcli.Client, cluster string) (dynamicHostIps []string, err error) { clusterinfo, err := cli.ClusterInfo(cluster) if err != nil { - return nil, fmt.Errorf("failed to get cluster details: %v", err) + // don't log error details from client calls in events + klog.V(4).Infof("failed to get cluster details: %v", err) + return nil, fmt.Errorf("failed to get cluster details: see kube-controller-manager.log for details") } // For the dynamically provisioned volume, we gather the list of node IPs @@ -964,7 +970,9 @@ func getClusterNodes(cli *gcli.Client, cluster string) (dynamicHostIps []string, for _, node := range clusterinfo.Nodes { nodeInfo, err := cli.NodeInfo(string(node)) if err != nil { - return nil, fmt.Errorf("failed to get host ipaddress: %v", err) + // don't log error details from client calls in events + klog.V(4).Infof("failed to get host ipaddress: %v", err) + return nil, fmt.Errorf("failed to get host ipaddress: see kube-controller-manager.log for details") } ipaddr := dstrings.Join(nodeInfo.NodeAddRequest.Hostnames.Storage, "") // IP validates if a string is a valid IP address. @@ -1213,8 +1221,9 @@ func (plugin *glusterfsPlugin) ExpandVolumeDevice(spec *volume.Spec, newSize res //Check the existing volume size currentVolumeInfo, err := cli.VolumeInfo(volumeID) if err != nil { - klog.Errorf("error when fetching details of volume %s: %v", volumeName, err) - return oldSize, err + // don't log error details from client calls in events + klog.V(4).Infof("error when fetching details of volume %s: %v", volumeName, err) + return oldSize, fmt.Errorf("failed to get volume info %s: see kube-controller-manager.log for details", volumeName) } if int64(currentVolumeInfo.Size) >= requestGiB { return newSize, nil @@ -1226,8 +1235,9 @@ func (plugin *glusterfsPlugin) ExpandVolumeDevice(spec *volume.Spec, newSize res // Expand the volume volumeInfoRes, err := cli.VolumeExpand(volumeID, volumeExpandReq) if err != nil { - klog.Errorf("failed to expand volume %s: %v", volumeName, err) - return oldSize, err + // don't log error details from client calls in events + klog.V(4).Infof("failed to expand volume %s: %v", volumeName, err) + return oldSize, fmt.Errorf("failed to expand volume: see kube-controller-manager.log for details") } klog.V(2).Infof("volume %s expanded to new size %d successfully", volumeName, volumeInfoRes.Size) newVolumeSize := resource.MustParse(fmt.Sprintf("%dGi", volumeInfoRes.Size)) diff --git a/pkg/volume/quobyte/quobyte.go b/pkg/volume/quobyte/quobyte.go index 33d730da522..a25ccaa6b4a 100644 --- a/pkg/volume/quobyte/quobyte.go +++ b/pkg/volume/quobyte/quobyte.go @@ -17,6 +17,7 @@ limitations under the License. package quobyte import ( + "errors" "fmt" "os" "path/filepath" @@ -413,7 +414,9 @@ func (provisioner *quobyteVolumeProvisioner) Provision(selectedNode *v1.Node, al vol, sizeGB, err := manager.createVolume(provisioner, createQuota) if err != nil { - return nil, err + // don't log error details from client calls in events + klog.V(4).Infof("CreateVolume failed: %v", err) + return nil, errors.New("CreateVolume failed: see kube-controller-manager.log for details") } pv := new(v1.PersistentVolume) metav1.SetMetaDataAnnotation(&pv.ObjectMeta, util.VolumeDynamicallyCreatedByKey, "quobyte-dynamic-provisioner") @@ -448,7 +451,13 @@ func (deleter *quobyteVolumeDeleter) Delete() error { manager := &quobyteVolumeManager{ config: cfg, } - return manager.deleteVolume(deleter) + err = manager.deleteVolume(deleter) + if err != nil { + // don't log error details from client calls in events + klog.V(4).Infof("DeleteVolume failed: %v", err) + return errors.New("DeleteVolume failed: see kube-controller-manager.log for details") + } + return nil } // Parse API configuration (url, username and password) out of class.Parameters. diff --git a/pkg/volume/scaleio/sio_client.go b/pkg/volume/scaleio/sio_client.go index 443a56f178c..111f6255ecf 100644 --- a/pkg/volume/scaleio/sio_client.go +++ b/pkg/volume/scaleio/sio_client.go @@ -126,8 +126,9 @@ func (c *sioClient) init() error { Username: c.username, Password: c.password}, ); err != nil { - klog.Error(log("client authentication failed: %v", err)) - return err + // don't log error details from client calls in events + klog.V(4).Infof(log("client authentication failed: %v", err)) + return errors.New("client authentication failed") } // retrieve system @@ -214,8 +215,9 @@ func (c *sioClient) CreateVolume(name string, sizeGB int64) (*siotypes.Volume, e } createResponse, err := c.client.CreateVolume(params, c.storagePool.Name) if err != nil { - klog.Error(log("failed to create volume %s: %v", name, err)) - return nil, err + // don't log error details from client calls in events + klog.V(4).Infof(log("failed to create volume %s: %v", name, err)) + return nil, errors.New("failed to create volume: see kubernetes logs for details") } return c.Volume(sioVolumeID(createResponse.ID)) } @@ -243,8 +245,9 @@ func (c *sioClient) AttachVolume(id sioVolumeID, multipleMappings bool) error { volClient.Volume = &siotypes.Volume{ID: string(id)} if err := volClient.MapVolumeSdc(params); err != nil { - klog.Error(log("failed to attach volume id %s: %v", id, err)) - return err + // don't log error details from client calls in events + klog.V(4).Infof(log("failed to attach volume id %s: %v", id, err)) + return errors.New("failed to attach volume: see kubernetes logs for details") } klog.V(4).Info(log("volume %s attached successfully", id)) @@ -269,7 +272,9 @@ func (c *sioClient) DetachVolume(id sioVolumeID) error { volClient := sio.NewVolume(c.client) volClient.Volume = &siotypes.Volume{ID: string(id)} if err := volClient.UnmapVolumeSdc(params); err != nil { - return err + // don't log error details from client calls in events + klog.V(4).Infof(log("failed to detach volume id %s: %v", id, err)) + return errors.New("failed to detach volume: see kubernetes logs for details") } return nil } @@ -287,7 +292,9 @@ func (c *sioClient) DeleteVolume(id sioVolumeID) error { volClient := sio.NewVolume(c.client) volClient.Volume = vol if err := volClient.RemoveVolume("ONLY_ME"); err != nil { - return err + // don't log error details from client calls in events + klog.V(4).Infof(log("failed to remove volume id %s: %v", id, err)) + return errors.New("failed to remove volume: see kubernetes logs for details") } return nil } @@ -306,8 +313,9 @@ func (c *sioClient) IID() (string, error) { } sdc, err := c.sysClient.FindSdc("SdcGUID", guid) if err != nil { - klog.Error(log("failed to retrieve sdc info %s", err)) - return "", err + // don't log error details from client calls in events + klog.V(4).Infof(log("failed to retrieve sdc info %s", err)) + return "", errors.New("failed to retrieve sdc info: see kubernetes logs for details") } c.instanceID = sdc.Sdc.ID klog.V(4).Info(log("retrieved instanceID %s", c.instanceID)) @@ -472,12 +480,15 @@ func (c *sioClient) WaitForDetachedDevice(token string) error { // *********************************************************************** func (c *sioClient) findSystem(sysname string) (sys *siotypes.System, err error) { if c.sysClient, err = c.client.FindSystem("", sysname, ""); err != nil { - return nil, err + // don't log error details from clients in events + klog.V(4).Infof(log("failed to find system %q: %v", sysname, err)) + return nil, errors.New("failed to find system: see kubernetes logs for details") } systems, err := c.client.GetInstance("") if err != nil { - klog.Error(log("failed to retrieve instances: %v", err)) - return nil, err + // don't log error details from clients in events + klog.V(4).Infof(log("failed to retrieve instances: %v", err)) + return nil, errors.New("failed to retrieve instances: see kubernetes logs for details") } for _, sys = range systems { if sys.Name == sysname { @@ -493,8 +504,9 @@ func (c *sioClient) findProtectionDomain(pdname string) (*siotypes.ProtectionDom if c.sysClient != nil { protectionDomain, err := c.sysClient.FindProtectionDomain("", pdname, "") if err != nil { - klog.Error(log("failed to retrieve protection domains: %v", err)) - return nil, err + // don't log error details from clients in events + klog.V(4).Infof(log("failed to retrieve protection domains: %v", err)) + return nil, errors.New("failed to retrieve protection domains: see kubernetes logs for details") } c.pdClient.ProtectionDomain = protectionDomain return protectionDomain, nil @@ -508,8 +520,9 @@ func (c *sioClient) findStoragePool(spname string) (*siotypes.StoragePool, error if c.pdClient != nil { sp, err := c.pdClient.FindStoragePool("", spname, "") if err != nil { - klog.Error(log("failed to retrieve storage pool: %v", err)) - return nil, err + // don't log error details from clients in events + klog.V(4).Infof(log("failed to retrieve storage pool: %v", err)) + return nil, errors.New("failed to retrieve storage pool: see kubernetes logs for details") } c.spClient.StoragePool = sp return sp, nil @@ -519,14 +532,32 @@ func (c *sioClient) findStoragePool(spname string) (*siotypes.StoragePool, error } func (c *sioClient) getVolumes() ([]*siotypes.Volume, error) { - return c.client.GetVolume("", "", "", "", true) + volumes, err := c.client.GetVolume("", "", "", "", true) + if err != nil { + // don't log error details from clients in events + klog.V(4).Infof(log("failed to get volumes: %v", err)) + return nil, errors.New("failed to get volumes: see kubernetes logs for details") + } + return volumes, nil } func (c *sioClient) getVolumesByID(id sioVolumeID) ([]*siotypes.Volume, error) { - return c.client.GetVolume("", string(id), "", "", true) + volumes, err := c.client.GetVolume("", string(id), "", "", true) + if err != nil { + // don't log error details from clients in events + klog.V(4).Infof(log("failed to get volumes by id: %v", err)) + return nil, errors.New("failed to get volumes by id: see kubernetes logs for details") + } + return volumes, nil } func (c *sioClient) getVolumesByName(name string) ([]*siotypes.Volume, error) { - return c.client.GetVolume("", "", "", name, true) + volumes, err := c.client.GetVolume("", "", "", name, true) + if err != nil { + // don't log error details from clients in events + klog.V(4).Infof(log("failed to get volumes by name: %v", err)) + return nil, errors.New("failed to get volumes by name: see kubernetes logs for details") + } + return volumes, nil } func (c *sioClient) getSdcPath() string { diff --git a/pkg/volume/storageos/storageos_util.go b/pkg/volume/storageos/storageos_util.go index 05dcf039c14..c71179e479a 100644 --- a/pkg/volume/storageos/storageos_util.go +++ b/pkg/volume/storageos/storageos_util.go @@ -127,8 +127,9 @@ func (u *storageosUtil) CreateVolume(p *storageosProvisioner) (*storageosVolume, vol, err := u.api.VolumeCreate(opts) if err != nil { - klog.Errorf("volume create failed for volume %q (%v)", opts.Name, err) - return nil, err + // don't log error details from client calls in events + klog.V(4).Infof("volume create failed for volume %q (%v)", opts.Name, err) + return nil, errors.New("volume create failed: see kube-controller-manager.log for details") } return &storageosVolume{ ID: vol.ID, @@ -293,7 +294,12 @@ func (u *storageosUtil) DeleteVolume(d *storageosDeleter) error { Namespace: d.volNamespace, Force: true, } - return u.api.VolumeDelete(opts) + if err := u.api.VolumeDelete(opts); err != nil { + // don't log error details from client calls in events + klog.V(4).Infof("volume deleted failed for volume %q in namespace %q: %v", d.volName, d.volNamespace, err) + return errors.New("volume delete failed: see kube-controller-manager.log for details") + } + return nil } // Get the node's device path from the API, falling back to the default if not