Add volumeattachment status subresource

pkg/apis/storage/validation/validation.go

@@ -218,6 +218,7 @@ func ValidateVolumeAttachmentUpdate(new, old *storage.VolumeAttachment) field.Er
 	allErrs := ValidateVolumeAttachment(new)
 
 	// Spec is read-only
+	// If this ever relaxes in the future, make sure to increment the Generation number in PrepareForUpdate
 	if !apiequality.Semantic.DeepEqual(old.Spec, new.Spec) {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec"), new.Spec, "field is immutable"))
 	}

pkg/registry/storage/rest/storage_storage.go

@@ -54,8 +54,8 @@ func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorag
 func (p RESTStorageProvider) v1alpha1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage {
 	storage := map[string]rest.Storage{}
 	// volumeattachments
-	volumeAttachmentStorage := volumeattachmentstore.NewREST(restOptionsGetter)
-	storage["volumeattachments"] = volumeAttachmentStorage
+	volumeAttachmentStorage := volumeattachmentstore.NewStorage(restOptionsGetter)
+	storage["volumeattachments"] = volumeAttachmentStorage.VolumeAttachment
 
 	return storage
 }
@@ -67,21 +67,24 @@ func (p RESTStorageProvider) v1beta1Storage(apiResourceConfigSource serverstorag
 	storage["storageclasses"] = storageClassStorage
 
 	// volumeattachments
-	volumeAttachmentStorage := volumeattachmentstore.NewREST(restOptionsGetter)
-	storage["volumeattachments"] = volumeAttachmentStorage
+	volumeAttachmentStorage := volumeattachmentstore.NewStorage(restOptionsGetter)
+	storage["volumeattachments"] = volumeAttachmentStorage.VolumeAttachment
 
 	return storage
 }
 
 func (p RESTStorageProvider) v1Storage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) map[string]rest.Storage {
-	storage := map[string]rest.Storage{}
-	// storageclasses
 	storageClassStorage := storageclassstore.NewREST(restOptionsGetter)
-	storage["storageclasses"] = storageClassStorage
+	volumeAttachmentStorage := volumeattachmentstore.NewStorage(restOptionsGetter)
 
-	// volumeattachments
-	volumeAttachmentStorage := volumeattachmentstore.NewREST(restOptionsGetter)
-	storage["volumeattachments"] = volumeAttachmentStorage
+	storage := map[string]rest.Storage{
+		// storageclasses
+		"storageclasses": storageClassStorage,
+
+		// volumeattachments
+		"volumeattachments":        volumeAttachmentStorage.VolumeAttachment,
+		"volumeattachments/status": volumeAttachmentStorage.Status,
+	}
 
 	return storage
 }

pkg/registry/storage/volumeattachment/BUILD

@@ -12,8 +12,11 @@ go_library(
         "//pkg/api/legacyscheme:go_default_library",
         "//pkg/apis/storage:go_default_library",
         "//pkg/apis/storage/validation:go_default_library",
+        "//staging/src/k8s.io/api/storage/v1beta1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library",
     ],
 )
@@ -24,7 +27,9 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//pkg/apis/storage:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
     ],
 )

pkg/registry/storage/volumeattachment/storage/BUILD

@@ -8,9 +8,11 @@ go_library(
     deps = [
         "//pkg/apis/storage:go_default_library",
         "//pkg/registry/storage/volumeattachment:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/registry/generic/registry:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
     ],
 )
 
@@ -19,17 +21,18 @@ go_test(
     srcs = ["storage_test.go"],
     embed = [":go_default_library"],
     deps = [
-        "//pkg/api/testapi:go_default_library",
         "//pkg/apis/storage:go_default_library",
         "//pkg/registry/registrytest:go_default_library",
-        "//staging/src/k8s.io/api/storage/v1alpha1:go_default_library",
-        "//staging/src/k8s.io/api/storage/v1beta1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/registry/generic/testing:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/storage/etcd/testing:go_default_library",
     ],
 )

pkg/registry/storage/volumeattachment/storage/storage.go

@@ -17,20 +17,30 @@ limitations under the License.
 package storage
 
 import (
+	"context"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
+	"k8s.io/apiserver/pkg/registry/rest"
 	storageapi "k8s.io/kubernetes/pkg/apis/storage"
 	"k8s.io/kubernetes/pkg/registry/storage/volumeattachment"
 )
 
-// REST object that will work against persistent volumes.
+// VolumeAttachmentStorage includes storage for VolumeAttachments and all subresources
+type VolumeAttachmentStorage struct {
+	VolumeAttachment *REST
+	Status           *StatusREST
+}
+
+// REST object that will work for VolumeAttachments
 type REST struct {
 	*genericregistry.Store
 }
 
-// NewREST returns a RESTStorage object that will work against persistent volumes.
-func NewREST(optsGetter generic.RESTOptionsGetter) *REST {
+// NewStorage returns a RESTStorage object that will work against VolumeAttachments
+func NewStorage(optsGetter generic.RESTOptionsGetter) *VolumeAttachmentStorage {
 	store := &genericregistry.Store{
 		NewFunc:                  func() runtime.Object { return &storageapi.VolumeAttachment{} },
 		NewListFunc:              func() runtime.Object { return &storageapi.VolumeAttachmentList{} },
@@ -46,5 +56,33 @@ func NewREST(optsGetter generic.RESTOptionsGetter) *REST {
 		panic(err) // TODO: Propagate error up
 	}
 
-	return &REST{store}
+	statusStore := *store
+	statusStore.UpdateStrategy = volumeattachment.StatusStrategy
+
+	return &VolumeAttachmentStorage{
+		VolumeAttachment: &REST{store},
+		Status:           &StatusREST{store: &statusStore},
+	}
+}
+
+// StatusREST implements the REST endpoint for changing the status of a VolumeAttachment
+type StatusREST struct {
+	store *genericregistry.Store
+}
+
+// New creates a new VolumeAttachment resource
+func (r *StatusREST) New() runtime.Object {
+	return &storageapi.VolumeAttachment{}
+}
+
+// Get retrieves the object from the storage. It is required to support Patch.
+func (r *StatusREST) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) {
+	return r.store.Get(ctx, name, options)
+}
+
+// Update alters the status subset of an object.
+func (r *StatusREST) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {
+	// We are explicitly setting forceAllowCreate to false in the call to the underlying storage because
+	// subresources should never allow create on update.
+	return r.store.Update(ctx, name, objInfo, createValidation, updateValidation, false, options)
 }

pkg/registry/storage/volumeattachment/storage/storage_test.go

@@ -19,21 +19,22 @@ package storage
 import (
 	"testing"
 
-	storageapiv1alpha1 "k8s.io/api/storage/v1alpha1"
-	storageapiv1beta1 "k8s.io/api/storage/v1beta1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/diff"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericregistrytest "k8s.io/apiserver/pkg/registry/generic/testing"
+	"k8s.io/apiserver/pkg/registry/rest"
 	etcdtesting "k8s.io/apiserver/pkg/storage/etcd/testing"
-	"k8s.io/kubernetes/pkg/api/testapi"
 	storageapi "k8s.io/kubernetes/pkg/apis/storage"
 	"k8s.io/kubernetes/pkg/registry/registrytest"
 )
 
-func newStorage(t *testing.T) (*REST, *etcdtesting.EtcdTestServer) {
+func newStorage(t *testing.T) (*REST, *StatusREST, *etcdtesting.EtcdTestServer) {
 	etcdStorage, server := registrytest.NewEtcdStorage(t, storageapi.GroupName)
 	restOptions := generic.RESTOptions{
 		StorageConfig:           etcdStorage,
@@ -41,8 +42,8 @@ func newStorage(t *testing.T) (*REST, *etcdtesting.EtcdTestServer) {
 		DeleteCollectionWorkers: 1,
 		ResourcePrefix:          "volumeattachments",
 	}
-	volumeAttachmentStorage := NewREST(restOptions)
-	return volumeAttachmentStorage, server
+	volumeAttachmentStorage := NewStorage(restOptions)
+	return volumeAttachmentStorage.VolumeAttachment, volumeAttachmentStorage.Status, server
 }
 
 func validNewVolumeAttachment(name string) *storageapi.VolumeAttachment {
@@ -62,13 +63,7 @@ func validNewVolumeAttachment(name string) *storageapi.VolumeAttachment {
 }
 
 func TestCreate(t *testing.T) {
-	if *testapi.Storage.GroupVersion() != storageapiv1alpha1.SchemeGroupVersion &&
-		*testapi.Storage.GroupVersion() != storageapiv1beta1.SchemeGroupVersion {
-		// skip the test for all versions exception v1alpha1 and v1beta1
-		return
-	}
-
-	storage, server := newStorage(t)
+	storage, _, server := newStorage(t)
 	defer server.Terminate(t)
 	defer storage.Store.DestroyFunc()
 	test := genericregistrytest.New(t, storage.Store).ClusterScope()
@@ -93,20 +88,16 @@ func TestCreate(t *testing.T) {
 }
 
 func TestUpdate(t *testing.T) {
-	if *testapi.Storage.GroupVersion() != storageapiv1alpha1.SchemeGroupVersion &&
-		*testapi.Storage.GroupVersion() != storageapiv1beta1.SchemeGroupVersion {
-		// skip the test for all versions exception v1alpha1 and v1beta1
-		return
-	}
-
-	storage, server := newStorage(t)
+	storage, _, server := newStorage(t)
 	defer server.Terminate(t)
 	defer storage.Store.DestroyFunc()
 	test := genericregistrytest.New(t, storage.Store).ClusterScope()
+
 	test.TestUpdate(
 		// valid
 		validNewVolumeAttachment("foo"),
-		// updateFunc
+		// we still allow status field to be set in both v1 and v1beta1
+		// it is just that in v1 the new value does not take effect.
 		func(obj runtime.Object) runtime.Object {
 			object := obj.(*storageapi.VolumeAttachment)
 			object.Status.Attached = true
@@ -122,13 +113,7 @@ func TestUpdate(t *testing.T) {
 }
 
 func TestDelete(t *testing.T) {
-	if *testapi.Storage.GroupVersion() != storageapiv1alpha1.SchemeGroupVersion &&
-		*testapi.Storage.GroupVersion() != storageapiv1beta1.SchemeGroupVersion {
-		// skip the test for all versions exception v1alpha1 and v1beta1
-		return
-	}
-
-	storage, server := newStorage(t)
+	storage, _, server := newStorage(t)
 	defer server.Terminate(t)
 	defer storage.Store.DestroyFunc()
 	test := genericregistrytest.New(t, storage.Store).ClusterScope().ReturnDeletedObject()
@@ -136,13 +121,7 @@ func TestDelete(t *testing.T) {
 }
 
 func TestGet(t *testing.T) {
-	if *testapi.Storage.GroupVersion() != storageapiv1alpha1.SchemeGroupVersion &&
-		*testapi.Storage.GroupVersion() != storageapiv1beta1.SchemeGroupVersion {
-		// skip the test for all versions exception v1alpha1 and v1beta1
-		return
-	}
-
-	storage, server := newStorage(t)
+	storage, _, server := newStorage(t)
 	defer server.Terminate(t)
 	defer storage.Store.DestroyFunc()
 	test := genericregistrytest.New(t, storage.Store).ClusterScope()
@@ -150,13 +129,7 @@ func TestGet(t *testing.T) {
 }
 
 func TestList(t *testing.T) {
-	if *testapi.Storage.GroupVersion() != storageapiv1alpha1.SchemeGroupVersion &&
-		*testapi.Storage.GroupVersion() != storageapiv1beta1.SchemeGroupVersion {
-		// skip the test for all versions exception v1alpha1 and v1beta1
-		return
-	}
-
-	storage, server := newStorage(t)
+	storage, _, server := newStorage(t)
 	defer server.Terminate(t)
 	defer storage.Store.DestroyFunc()
 	test := genericregistrytest.New(t, storage.Store).ClusterScope()
@@ -164,13 +137,7 @@ func TestList(t *testing.T) {
 }
 
 func TestWatch(t *testing.T) {
-	if *testapi.Storage.GroupVersion() != storageapiv1alpha1.SchemeGroupVersion &&
-		*testapi.Storage.GroupVersion() != storageapiv1beta1.SchemeGroupVersion {
-		// skip the test for all versions exception v1alpha1 and v1beta1
-		return
-	}
-
-	storage, server := newStorage(t)
+	storage, _, server := newStorage(t)
 	defer server.Terminate(t)
 	defer storage.Store.DestroyFunc()
 	test := genericregistrytest.New(t, storage.Store).ClusterScope()
@@ -192,3 +159,41 @@ func TestWatch(t *testing.T) {
 		},
 	)
 }
+
+func TestEtcdStatusUpdate(t *testing.T) {
+	storage, statusStorage, server := newStorage(t)
+	defer server.Terminate(t)
+	defer storage.Store.DestroyFunc()
+	ctx := genericapirequest.NewDefaultContext()
+
+	attachment := validNewVolumeAttachment("foo")
+	if _, err := storage.Create(ctx, attachment, rest.ValidateAllObjectFunc, &metav1.CreateOptions{}); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	obj, err := storage.Get(ctx, attachment.ObjectMeta.Name, &metav1.GetOptions{})
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	// update status
+	attachmentIn := obj.(*storageapi.VolumeAttachment).DeepCopy()
+	attachmentIn.Status.Attached = true
+
+	_, _, err = statusStorage.Update(ctx, attachmentIn.Name, rest.DefaultUpdatedObjectInfo(attachmentIn), rest.ValidateAllObjectFunc, rest.ValidateAllObjectUpdateFunc, false, &metav1.UpdateOptions{})
+	if err != nil {
+		t.Fatalf("Failed to update status: %v", err)
+	}
+
+	// validate object got updated
+	obj, err = storage.Get(ctx, attachmentIn.ObjectMeta.Name, &metav1.GetOptions{})
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	attachmentOut := obj.(*storageapi.VolumeAttachment)
+	if !apiequality.Semantic.DeepEqual(attachmentIn.Spec, attachmentOut.Spec) {
+		t.Errorf("objects differ: %v", diff.ObjectDiff(attachmentOut.Spec, attachmentIn.Spec))
+	}
+	if !apiequality.Semantic.DeepEqual(attachmentIn.Status, attachmentOut.Status) {
+		t.Errorf("objects differ: %v", diff.ObjectDiff(attachmentOut.Status, attachmentIn.Status))
+	}
+}

pkg/registry/storage/volumeattachment/strategy.go

@@ -19,8 +19,11 @@ package volumeattachment
 import (
 	"context"
 
+	storageapiv1beta1 "k8s.io/api/storage/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/storage/names"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/apis/storage"
@@ -43,6 +46,19 @@ func (volumeAttachmentStrategy) NamespaceScoped() bool {
 
 // ResetBeforeCreate clears the Status field which is not allowed to be set by end users on creation.
 func (volumeAttachmentStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
+	var groupVersion schema.GroupVersion
+
+	if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found {
+		groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion}
+	}
+
+	switch groupVersion {
+	case storageapiv1beta1.SchemeGroupVersion:
+		// allow modification of status for v1beta1
+	default:
+		volumeAttachment := obj.(*storage.VolumeAttachment)
+		volumeAttachment.Status = storage.VolumeAttachmentStatus{}
+	}
 }
 
 func (volumeAttachmentStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
@@ -58,8 +74,22 @@ func (volumeAttachmentStrategy) AllowCreateOnUpdate() bool {
 	return false
 }
 
-// PrepareForUpdate sets the Status fields which is not allowed to be set by an end user updating a PV
+// PrepareForUpdate sets the Status fields which is not allowed to be set by an end user updating a VolumeAttachment
 func (volumeAttachmentStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
+	var groupVersion schema.GroupVersion
+
+	if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found {
+		groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion}
+	}
+	switch groupVersion {
+	case storageapiv1beta1.SchemeGroupVersion:
+		// allow modification of Status via main resource for v1beta1
+	default:
+		newVolumeAttachment := obj.(*storage.VolumeAttachment)
+		oldVolumeAttachment := old.(*storage.VolumeAttachment)
+		newVolumeAttachment.Status = oldVolumeAttachment.Status
+		// No need to increment Generation because we don't allow updates to spec
+	}
 }
 
 func (volumeAttachmentStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
@@ -72,3 +102,30 @@ func (volumeAttachmentStrategy) ValidateUpdate(ctx context.Context, obj, old run
 func (volumeAttachmentStrategy) AllowUnconditionalUpdate() bool {
 	return false
 }
+
+// volumeAttachmentStatusStrategy implements behavior for VolumeAttachmentStatus subresource
+type volumeAttachmentStatusStrategy struct {
+	volumeAttachmentStrategy
+}
+
+// StatusStrategy is the default logic that applies when creating and updating
+// VolumeAttachmentStatus subresource via the REST API.
+var StatusStrategy = volumeAttachmentStatusStrategy{Strategy}
+
+// PrepareForUpdate sets the Status fields which is not allowed to be set by an end user updating a VolumeAttachment
+func (volumeAttachmentStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
+	newVolumeAttachment := obj.(*storage.VolumeAttachment)
+	oldVolumeAttachment := old.(*storage.VolumeAttachment)
+
+	newVolumeAttachment.Spec = oldVolumeAttachment.Spec
+
+	oldMeta := oldVolumeAttachment.ObjectMeta
+	newMeta := &newVolumeAttachment.ObjectMeta
+	newMeta.SetDeletionTimestamp(oldMeta.GetDeletionTimestamp())
+	newMeta.SetGeneration(oldMeta.GetGeneration())
+	newMeta.SetSelfLink(oldMeta.GetSelfLink())
+	newMeta.SetLabels(oldMeta.GetLabels())
+	newMeta.SetAnnotations(oldMeta.GetAnnotations())
+	newMeta.SetFinalizers(oldMeta.GetFinalizers())
+	newMeta.SetOwnerReferences(oldMeta.GetOwnerReferences())
+}

pkg/registry/storage/volumeattachment/strategy_test.go

@@ -19,33 +19,42 @@ package volumeattachment
 import (
 	"testing"
 
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/diff"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/kubernetes/pkg/apis/storage"
 )
 
-func TestVolumeAttachmentStrategy(t *testing.T) {
-	ctx := genericapirequest.NewDefaultContext()
-	if Strategy.NamespaceScoped() {
-		t.Errorf("VolumeAttachment must not be namespace scoped")
-	}
-	if Strategy.AllowCreateOnUpdate() {
-		t.Errorf("VolumeAttachment should not allow create on update")
-	}
-
-	pvName := "name"
-	volumeAttachment := &storage.VolumeAttachment{
+func getValidVolumeAttachment(name string) *storage.VolumeAttachment {
+	return &storage.VolumeAttachment{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "valid-attachment",
+			Name: name,
 		},
 		Spec: storage.VolumeAttachmentSpec{
 			Attacher: "valid-attacher",
 			Source: storage.VolumeAttachmentSource{
-				PersistentVolumeName: &pvName,
+				PersistentVolumeName: &name,
 			},
 			NodeName: "valid-node",
 		},
 	}
+}
+
+func TestVolumeAttachmentStrategy(t *testing.T) {
+	ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{
+		APIGroup:   "storage.k8s.io",
+		APIVersion: "v1",
+		Resource:   "volumeattachments",
+	})
+	if Strategy.NamespaceScoped() {
+		t.Errorf("VolumeAttachment must not be namespace scoped")
+	}
+	if Strategy.AllowCreateOnUpdate() {
+		t.Errorf("VolumeAttachment should not allow create on update")
+	}
+
+	volumeAttachment := getValidVolumeAttachment("valid-attachment")
 
 	Strategy.PrepareForCreate(ctx, volumeAttachment)
 
@@ -54,19 +63,18 @@ func TestVolumeAttachmentStrategy(t *testing.T) {
 		t.Errorf("unexpected error validating %v", errs)
 	}
 
-	newVolumeAttachment := &storage.VolumeAttachment{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "valid-attachment-2",
-		},
-		Spec: storage.VolumeAttachmentSpec{
-			Attacher: "valid-attacher-2",
-			Source: storage.VolumeAttachmentSource{
-				PersistentVolumeName: &pvName,
-			},
-			NodeName: "valid-node-2",
-		},
+	// Create with status should drop status
+	statusVolumeAttachment := volumeAttachment.DeepCopy()
+	statusVolumeAttachment.Status = storage.VolumeAttachmentStatus{Attached: true}
+	Strategy.PrepareForCreate(ctx, statusVolumeAttachment)
+	if !apiequality.Semantic.DeepEqual(statusVolumeAttachment, volumeAttachment) {
+		t.Errorf("unexpected objects difference after creating with status: %v", diff.ObjectDiff(statusVolumeAttachment, volumeAttachment))
 	}
 
+	// Update of spec is disallowed
+	newVolumeAttachment := volumeAttachment.DeepCopy()
+	newVolumeAttachment.Spec.NodeName = "valid-node-2"
+
 	Strategy.PrepareForUpdate(ctx, newVolumeAttachment, volumeAttachment)
 
 	errs = Strategy.ValidateUpdate(ctx, newVolumeAttachment, volumeAttachment)
@@ -74,4 +82,116 @@ func TestVolumeAttachmentStrategy(t *testing.T) {
 		t.Errorf("Expected a validation error")
 	}
 
+	// modifying status should be dropped
+	statusVolumeAttachment = volumeAttachment.DeepCopy()
+	statusVolumeAttachment.Status = storage.VolumeAttachmentStatus{Attached: true}
+
+	Strategy.PrepareForUpdate(ctx, statusVolumeAttachment, volumeAttachment)
+
+	if !apiequality.Semantic.DeepEqual(statusVolumeAttachment, volumeAttachment) {
+		t.Errorf("unexpected objects difference after modfying status: %v", diff.ObjectDiff(statusVolumeAttachment, volumeAttachment))
+	}
+}
+
+func TestVolumeAttachmentStatusStrategy(t *testing.T) {
+	ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{
+		APIGroup:   "storage.k8s.io",
+		APIVersion: "v1",
+		Resource:   "volumeattachments",
+	})
+
+	volumeAttachment := getValidVolumeAttachment("valid-attachment")
+
+	// modifying status should be allowed
+	statusVolumeAttachment := volumeAttachment.DeepCopy()
+	statusVolumeAttachment.Status = storage.VolumeAttachmentStatus{Attached: true}
+
+	expectedVolumeAttachment := statusVolumeAttachment.DeepCopy()
+	StatusStrategy.PrepareForUpdate(ctx, statusVolumeAttachment, volumeAttachment)
+	if !apiequality.Semantic.DeepEqual(statusVolumeAttachment, expectedVolumeAttachment) {
+		t.Errorf("unexpected objects differerence after modifying status: %v", diff.ObjectDiff(statusVolumeAttachment, expectedVolumeAttachment))
+	}
+
+	// modifying spec should be dropped
+	newVolumeAttachment := volumeAttachment.DeepCopy()
+	newVolumeAttachment.Spec.NodeName = "valid-node-2"
+
+	StatusStrategy.PrepareForUpdate(ctx, newVolumeAttachment, volumeAttachment)
+	if !apiequality.Semantic.DeepEqual(newVolumeAttachment, volumeAttachment) {
+		t.Errorf("unexpected objects differerence after modifying spec: %v", diff.ObjectDiff(newVolumeAttachment, volumeAttachment))
+	}
+}
+
+func TestBetaAndV1StatusUpdate(t *testing.T) {
+	tests := []struct {
+		requestInfo    genericapirequest.RequestInfo
+		newStatus      bool
+		expectedStatus bool
+	}{
+		{
+			genericapirequest.RequestInfo{
+				APIGroup:   "storage.k8s.io",
+				APIVersion: "v1",
+				Resource:   "volumeattachments",
+			},
+			true,
+			false,
+		},
+		{
+			genericapirequest.RequestInfo{
+				APIGroup:   "storage.k8s.io",
+				APIVersion: "v1beta1",
+				Resource:   "volumeattachments",
+			},
+			true,
+			true,
+		},
+	}
+	for _, test := range tests {
+		va := getValidVolumeAttachment("valid-attachment")
+		newAttachment := va.DeepCopy()
+		newAttachment.Status.Attached = test.newStatus
+		context := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &test.requestInfo)
+		Strategy.PrepareForUpdate(context, newAttachment, va)
+		if newAttachment.Status.Attached != test.expectedStatus {
+			t.Errorf("expected status to be %v got %v", test.expectedStatus, newAttachment.Status.Attached)
+		}
+	}
+
+}
+
+func TestBetaAndV1StatusCreate(t *testing.T) {
+	tests := []struct {
+		requestInfo    genericapirequest.RequestInfo
+		newStatus      bool
+		expectedStatus bool
+	}{
+		{
+			genericapirequest.RequestInfo{
+				APIGroup:   "storage.k8s.io",
+				APIVersion: "v1",
+				Resource:   "volumeattachments",
+			},
+			true,
+			false,
+		},
+		{
+			genericapirequest.RequestInfo{
+				APIGroup:   "storage.k8s.io",
+				APIVersion: "v1beta1",
+				Resource:   "volumeattachments",
+			},
+			true,
+			true,
+		},
+	}
+	for _, test := range tests {
+		va := getValidVolumeAttachment("valid-attachment")
+		va.Status.Attached = test.newStatus
+		context := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &test.requestInfo)
+		Strategy.PrepareForCreate(context, va)
+		if va.Status.Attached != test.expectedStatus {
+			t.Errorf("expected status to be %v got %v", test.expectedStatus, va.Status.Attached)
+		}
+	}
 }

pkg/volume/csi/csi_attacher.go

@@ -95,7 +95,6 @@ func (c *csiAttacher) Attach(spec *volume.Spec, nodeName types.NodeName) (string
 				PersistentVolumeName: &pvName,
 			},
 		},
-		Status: storage.VolumeAttachmentStatus{Attached: false},
 	}
 
 	_, err = c.k8s.StorageV1beta1().VolumeAttachments().Create(attachment)