Skip to content

Commit

Permalink
Merge pull request #27714 from jsafrane/event-recycle
Browse files Browse the repository at this point in the history 
Automatic merge from submit-queue

Send recycle events from pod to pv.

This allows users to diagnose what's wrong with recycler. Recycler pods are started automatically with a cryptic name and they are deleted immediately when they finish.

e.g, `kubectl describe pv` could show that NFS cannot be mounted (and how many pods have tried it):

```
  FirstSeen     LastSeen        Count   From                            SubobjectPath   Type            Reason          Message
  ---------     --------        -----   ----                            -------------   --------        ------          -------
  59m           59m             1       {persistentvolume-controller }                  Warning         RecyclerPod     Recycler pod: Unable to mount volumes for pod "recycler-for-nfs_default(5421800e-347b-11e6-a79b-3c970e965218)": timeout expired waiting for volumes to attach/mount for pod "recycler-for-nfs"/"default". list of unattached/unmounted volumes=[vol]
  53m           53m             1       {persistentvolume-controller }                  Warning         RecyclerPod     Recycler pod: Unable to mount volumes for pod "recycler-for-nfs_default(3c9809e5-347c-11e6-a79b-3c970e965218)": timeout expired waiting for volumes to attach/mount for pod "recycler-for-nfs"/"default". list of unattached/unmounted volumes=[vol]
  46m           46m             1       {persistentvolume-controller }                  Warning         RecyclerPod     Recycler pod: Unable to mount volumes for pod "recycler-for-nfs_default(250dd2a2-347d-11e6-a79b-3c970e965218)": timeout expired waiting for volumes to attach/mount for pod "recycler-for-nfs"/"default". list of unattached/unmounted volumes=[vol]
  40m           40m             1       {persistentvolume-controller }                  Warning         RecyclerPod     Recycler pod: Unable to mount volumes for pod "recycler-for-nfs_default(0d84ea33-347e-11e6-a79b-3c970e965218)": timeout expired waiting for volumes to attach/mount for pod "recycler-for-nfs"/"default". list of unattached/unmounted volumes=[vol]
  33m           33m             1       {persistentvolume-controller }                  Warning         RecyclerPod     Recycler pod: Unable to mount volumes for pod "recycler-for-nfs_default(f5fb63bf-347e-11e6-a79b-3c970e965218)": timeout expired waiting for volumes to attach/mount for pod "recycler-for-nfs"/"default". list of unattached/unmounted volumes=[vol]
  27m           27m             1       {persistentvolume-controller }                  Warning         RecyclerPod     Recycler pod: Unable to mount volumes for pod "recycler-for-nfs_default(de7128fd-347f-11e6-a79b-3c970e965218)": timeout expired waiting for volumes to attach/mount for pod "recycler-for-nfs"/"default". list of unattached/unmounted volumes=[vol]
  1h            3m              75      {persistentvolume-controller }                  Normal          RecyclerPod     Recycler pod: Successfully assigned recycler-for-nfs to 127.0.0.1
  1h            3m              76      {persistentvolume-controller }                  Normal          RecyclerPod     Recycler pod: Pod was active on the node longer than specified deadline
  1h            1m              12      {persistentvolume-controller }                  Warning         RecyclerPod     Recycler pod: Error syncing pod, skipping: timeout expired waiting for volumes to attach/mount for pod "recycler-for-nfs"/"default". list of unattached/unmounted volumes=[vol]
  20m           1m              4       {persistentvolume-controller }                  Warning         RecyclerPod     (events with common reason combined)
```

These steps were necessary:

- added event watcher to volume.RecycleVolumeByWatchingPodUntilCompletion
- pass all these events through volume plugins to volume controller
- rework volume.RecycleVolumeByWatchingPodUntilCompletion unit tests to a table (too much copy-paste)
- fix all unit tests along the way
  • Loading branch information
Kubernetes Submit Queue authored Sep 22, 2016
2 parents 4ab5a76 + d7111b2 commit e9f4db2
Show file tree
Hide file tree
Showing 10 changed files with 341 additions and 160 deletions.
14 changes: 12 additions & 2 deletions pkg/controller/volume/persistentvolume/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,6 @@ func (ctrl *PersistentVolumeController) unbindVolume(volume *api.PersistentVolum
// Update the status
_, err = ctrl.updateVolumePhase(newVol, api.VolumeAvailable, "")
return err

}

// reclaimVolume implements volume.Spec.PersistentVolumeReclaimPolicy and
Expand Down Expand Up @@ -996,7 +995,8 @@ func (ctrl *PersistentVolumeController) recycleVolumeOperation(arg interface{})
}

// Plugin found
recycler, err := plugin.NewRecycler(volume.Name, spec)
recorder := ctrl.newRecyclerEventRecorder(volume)
recycler, err := plugin.NewRecycler(volume.Name, spec, recorder)
if err != nil {
// Cannot create recycler
strerr := fmt.Sprintf("Failed to create recycler: %v", err)
Expand Down Expand Up @@ -1024,6 +1024,8 @@ func (ctrl *PersistentVolumeController) recycleVolumeOperation(arg interface{})
}

glog.V(2).Infof("volume %q recycled", volume.Name)
// Send an event
ctrl.eventRecorder.Event(volume, api.EventTypeNormal, "VolumeRecycled", "Volume recycled")
// Make the volume available again
if err = ctrl.unbindVolume(volume); err != nil {
// Oops, could not save the volume and therefore the controller will
Expand Down Expand Up @@ -1381,6 +1383,14 @@ func (ctrl *PersistentVolumeController) scheduleOperation(operationName string,
}
}

// newRecyclerEventRecorder returns a RecycleEventRecorder that sends all events
// to given volume.
func (ctrl *PersistentVolumeController) newRecyclerEventRecorder(volume *api.PersistentVolume) vol.RecycleEventRecorder {
return func(eventtype, message string) {
ctrl.eventRecorder.Eventf(volume, eventtype, "RecyclerPod", "Recycler pod: %s", message)
}
}

// findProvisionablePlugin finds a provisioner plugin for a given claim.
// It returns either the provisioning plugin or nil when an external
// provisioner is requested.
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/volume/persistentvolume/framework_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ func (plugin *mockVolumePlugin) GetMetrics() (*vol.Metrics, error) {

// Recycler interfaces

func (plugin *mockVolumePlugin) NewRecycler(pvName string, spec *vol.Spec) (vol.Recycler, error) {
func (plugin *mockVolumePlugin) NewRecycler(pvName string, spec *vol.Spec, eventRecorder vol.RecycleEventRecorder) (vol.Recycler, error) {
if len(plugin.recycleCalls) > 0 {
// mockVolumePlugin directly implements Recycler interface
glog.V(4).Infof("mock plugin NewRecycler called, returning mock recycler")
Expand Down
26 changes: 14 additions & 12 deletions pkg/volume/host_path/host_path.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func ProbeVolumePlugins(volumeConfig volume.VolumeConfig) []volume.VolumePlugin
type hostPathPlugin struct {
host volume.VolumeHost
// decouple creating Recyclers/Deleters/Provisioners by deferring to a function. Allows for easier testing.
newRecyclerFunc func(pvName string, spec *volume.Spec, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error)
newRecyclerFunc func(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error)
newDeleterFunc func(spec *volume.Spec, host volume.VolumeHost) (volume.Deleter, error)
newProvisionerFunc func(options volume.VolumeOptions, host volume.VolumeHost) (volume.Provisioner, error)
config volume.VolumeConfig
Expand Down Expand Up @@ -112,8 +112,8 @@ func (plugin *hostPathPlugin) NewUnmounter(volName string, podUID types.UID) (vo
}}, nil
}

func (plugin *hostPathPlugin) NewRecycler(pvName string, spec *volume.Spec) (volume.Recycler, error) {
return plugin.newRecyclerFunc(pvName, spec, plugin.host, plugin.config)
func (plugin *hostPathPlugin) NewRecycler(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder) (volume.Recycler, error) {
return plugin.newRecyclerFunc(pvName, spec, eventRecorder, plugin.host, plugin.config)
}

func (plugin *hostPathPlugin) NewDeleter(spec *volume.Spec) (volume.Deleter, error) {
Expand Down Expand Up @@ -142,18 +142,19 @@ func (plugin *hostPathPlugin) ConstructVolumeSpec(volumeName, mountPath string)
return volume.NewSpecFromVolume(hostPathVolume), nil
}

func newRecycler(pvName string, spec *volume.Spec, host volume.VolumeHost, config volume.VolumeConfig) (volume.Recycler, error) {
func newRecycler(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder, host volume.VolumeHost, config volume.VolumeConfig) (volume.Recycler, error) {
if spec.PersistentVolume == nil || spec.PersistentVolume.Spec.HostPath == nil {
return nil, fmt.Errorf("spec.PersistentVolumeSource.HostPath is nil")
}
path := spec.PersistentVolume.Spec.HostPath.Path
return &hostPathRecycler{
name: spec.Name(),
path: path,
host: host,
config: config,
timeout: volume.CalculateTimeoutForVolume(config.RecyclerMinimumTimeout, config.RecyclerTimeoutIncrement, spec.PersistentVolume),
pvName: pvName,
name: spec.Name(),
path: path,
host: host,
config: config,
timeout: volume.CalculateTimeoutForVolume(config.RecyclerMinimumTimeout, config.RecyclerTimeoutIncrement, spec.PersistentVolume),
pvName: pvName,
eventRecorder: eventRecorder,
}, nil
}

Expand Down Expand Up @@ -234,7 +235,8 @@ type hostPathRecycler struct {
config volume.VolumeConfig
timeout int64
volume.MetricsNil
pvName string
pvName string
eventRecorder volume.RecycleEventRecorder
}

func (r *hostPathRecycler) GetPath() string {
Expand All @@ -253,7 +255,7 @@ func (r *hostPathRecycler) Recycle() error {
Path: r.path,
},
}
return volume.RecycleVolumeByWatchingPodUntilCompletion(r.pvName, pod, r.host.GetKubeClient())
return volume.RecycleVolumeByWatchingPodUntilCompletion(r.pvName, pod, r.host.GetKubeClient(), r.eventRecorder)
}

// hostPathProvisioner implements a Provisioner for the HostPath plugin
Expand Down
2 changes: 1 addition & 1 deletion pkg/volume/host_path/host_path_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func TestRecycler(t *testing.T) {
if err != nil {
t.Errorf("Can't find the plugin by name")
}
recycler, err := plug.NewRecycler("pv-name", spec)
recycler, err := plug.NewRecycler("pv-name", spec, nil)
if err != nil {
t.Errorf("Failed to make a new Recyler: %v", err)
}
Expand Down
28 changes: 15 additions & 13 deletions pkg/volume/nfs/nfs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func ProbeVolumePlugins(volumeConfig volume.VolumeConfig) []volume.VolumePlugin
type nfsPlugin struct {
host volume.VolumeHost
// decouple creating recyclers by deferring to a function. Allows for easier testing.
newRecyclerFunc func(pvName string, spec *volume.Spec, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error)
newRecyclerFunc func(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error)
config volume.VolumeConfig
}

Expand Down Expand Up @@ -132,8 +132,8 @@ func (plugin *nfsPlugin) newUnmounterInternal(volName string, podUID types.UID,
}}, nil
}

func (plugin *nfsPlugin) NewRecycler(pvName string, spec *volume.Spec) (volume.Recycler, error) {
return plugin.newRecyclerFunc(pvName, spec, plugin.host, plugin.config)
func (plugin *nfsPlugin) NewRecycler(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder) (volume.Recycler, error) {
return plugin.newRecyclerFunc(pvName, spec, eventRecorder, plugin.host, plugin.config)
}

func (plugin *nfsPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
Expand Down Expand Up @@ -274,18 +274,19 @@ func (c *nfsUnmounter) TearDownAt(dir string) error {
return nil
}

func newRecycler(pvName string, spec *volume.Spec, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error) {
func newRecycler(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error) {
if spec.PersistentVolume == nil || spec.PersistentVolume.Spec.NFS == nil {
return nil, fmt.Errorf("spec.PersistentVolumeSource.NFS is nil")
}
return &nfsRecycler{
name: spec.Name(),
server: spec.PersistentVolume.Spec.NFS.Server,
path: spec.PersistentVolume.Spec.NFS.Path,
host: host,
config: volumeConfig,
timeout: volume.CalculateTimeoutForVolume(volumeConfig.RecyclerMinimumTimeout, volumeConfig.RecyclerTimeoutIncrement, spec.PersistentVolume),
pvName: pvName,
name: spec.Name(),
server: spec.PersistentVolume.Spec.NFS.Server,
path: spec.PersistentVolume.Spec.NFS.Path,
host: host,
config: volumeConfig,
timeout: volume.CalculateTimeoutForVolume(volumeConfig.RecyclerMinimumTimeout, volumeConfig.RecyclerTimeoutIncrement, spec.PersistentVolume),
pvName: pvName,
eventRecorder: eventRecorder,
}, nil
}

Expand All @@ -298,7 +299,8 @@ type nfsRecycler struct {
config volume.VolumeConfig
timeout int64
volume.MetricsNil
pvName string
pvName string
eventRecorder volume.RecycleEventRecorder
}

func (r *nfsRecycler) GetPath() string {
Expand All @@ -318,7 +320,7 @@ func (r *nfsRecycler) Recycle() error {
Path: r.path,
},
}
return volume.RecycleVolumeByWatchingPodUntilCompletion(r.pvName, pod, r.host.GetKubeClient())
return volume.RecycleVolumeByWatchingPodUntilCompletion(r.pvName, pod, r.host.GetKubeClient(), r.eventRecorder)
}

func getVolumeSource(spec *volume.Spec) (*api.NFSVolumeSource, bool, error) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/nfs/nfs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func TestRecycler(t *testing.T) {
if err != nil {
t.Errorf("Can't find the plugin by name")
}
recycler, err := plug.NewRecycler("pv-name", spec)
recycler, err := plug.NewRecycler("pv-name", spec, nil)
if err != nil {
t.Errorf("Failed to make a new Recyler: %v", err)
}
Expand All @@ -103,7 +103,7 @@ func TestRecycler(t *testing.T) {
}
}

func newMockRecycler(pvName string, spec *volume.Spec, host volume.VolumeHost, config volume.VolumeConfig) (volume.Recycler, error) {
func newMockRecycler(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder, host volume.VolumeHost, config volume.VolumeConfig) (volume.Recycler, error) {
return &mockRecycler{
path: spec.PersistentVolume.Spec.NFS.Path,
}, nil
Expand Down
9 changes: 6 additions & 3 deletions pkg/volume/plugins.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,12 @@ type PersistentVolumePlugin interface {
// again to new claims
type RecyclableVolumePlugin interface {
VolumePlugin
// NewRecycler creates a new volume.Recycler which knows how to reclaim
// this resource after the volume's release from a PersistentVolumeClaim
NewRecycler(pvName string, spec *Spec) (Recycler, error)
// NewRecycler creates a new volume.Recycler which knows how to reclaim this
// resource after the volume's release from a PersistentVolumeClaim. The
// recycler will use the provided recorder to write any events that might be
// interesting to user. It's expected that caller will pass these events to
// the PV being recycled.
NewRecycler(pvName string, spec *Spec, eventRecorder RecycleEventRecorder) (Recycler, error)
}

// DeletableVolumePlugin is an extended interface of VolumePlugin and is used
Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/testing/testing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ func (plugin *FakeVolumePlugin) GetNewDetacherCallCount() int {
return plugin.NewDetacherCallCount
}

func (plugin *FakeVolumePlugin) NewRecycler(pvName string, spec *Spec) (Recycler, error) {
func (plugin *FakeVolumePlugin) NewRecycler(pvName string, spec *Spec, eventRecorder RecycleEventRecorder) (Recycler, error) {
return &fakeRecycler{"/attributesTransferredFromSpec", MetricsNil{}}, nil
}

Expand Down Expand Up @@ -457,7 +457,7 @@ func (fr *fakeRecycler) GetPath() string {
return fr.path
}

func NewFakeRecycler(pvName string, spec *Spec, host VolumeHost, config VolumeConfig) (Recycler, error) {
func NewFakeRecycler(pvName string, spec *Spec, eventRecorder RecycleEventRecorder, host VolumeHost, config VolumeConfig) (Recycler, error) {
if spec.PersistentVolume == nil || spec.PersistentVolume.Spec.HostPath == nil {
return nil, fmt.Errorf("fakeRecycler only supports spec.PersistentVolume.Spec.HostPath")
}
Expand Down
Loading

0 comments on commit e9f4db2

Please sign in to comment.