Skip to content

Commit

Permalink
Merge branch 'dev' into aws-sdk-go-v2/credentials
Browse files Browse the repository at this point in the history
  • Loading branch information
@tinnywang
tinnywang authored Nov 14, 2024
2 parents 3a636c9 + 9494fb3 commit 79aa903
Show file tree
Hide file tree
Showing 6 changed files with 196 additions and 58 deletions.
13 changes: 12 additions & 1 deletion ecs-init/volumes/ecs_volume_plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,17 @@ func (a *AmazonECSVolumePlugin) LoadState() error {
if oldState.Volumes == nil {
return nil
}

// Reset volume mount reference count. This is for backwards-compatibility with old
// state file format which did not have reference counting of volume mounts.
for _, vol := range oldState.Volumes {
for mountId, count := range vol.Mounts {
if count == 0 {
vol.Mounts[mountId] = 1
}
}
}

for volName, vol := range oldState.Volumes {
voldriver, err := a.getVolumeDriver(vol.Type)
if err != nil {
Expand Down Expand Up @@ -146,7 +157,7 @@ func (a *AmazonECSVolumePlugin) Create(r *volume.CreateRequest) error {
Path: target,
Options: r.Options,
CreatedAt: time.Now().Format(time.RFC3339Nano),
Mounts: map[string]*string{},
Mounts: map[string]int{},
}
// record the volume information
a.volumes[r.Name] = vol
Expand Down
164 changes: 123 additions & 41 deletions ecs-init/volumes/ecs_volume_plugin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/docker/go-plugins-helpers/volume"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestVolumeDriver implements VolumeDriver interface for testing
Expand Down Expand Up @@ -611,35 +612,95 @@ func TestCapabilities(t *testing.T) {
}

func TestPluginLoadState(t *testing.T) {
plugin := &AmazonECSVolumePlugin{
volumeDrivers: map[string]driver.VolumeDriver{
"efs": NewECSVolumeDriver(),
tcs := []struct {
name string
stateFileContents string
pluginAssertions func(*testing.T, *AmazonECSVolumePlugin)
}{
{
name: "backwards compatibility with state format without reference counting of mounts",
stateFileContents: `
{
"volumes": {
"efsVolume": {
"type":"efs",
"path":"/var/lib/ecs/volumes/efsVolume",
"options": {"device":"fs-123","o":"tls","type":"efs"},
"mounts": {"id1": null}
}
}
}`,
pluginAssertions: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
assert.Len(t, plugin.volumes, 1)
vol, ok := plugin.volumes["efsVolume"]
assert.True(t, ok)
assert.Equal(t, "efs", vol.Type)
assert.Equal(t, VolumeMountPathPrefix+"efsVolume", vol.Path)
vols := plugin.state.VolState.Volumes
assert.Len(t, vols, 1)
volInfo, ok := vols["efsVolume"]
require.True(t, ok)
assert.Equal(t, "efs", volInfo.Type)
assert.Equal(t, VolumeMountPathPrefix+"efsVolume", volInfo.Path)

// Test for backwards compatibility of old state format following implementation of
// reference counting of volume mounts null value for mount IDs should be converted to 1.
assert.Equal(t, map[string]int{"id1": 1}, vols["efsVolume"].Mounts)
},
},
{
name: "current state format",
stateFileContents: `
{
"volumes": {
"efsVolume": {
"type":"efs",
"path":"/var/lib/ecs/volumes/efsVolume",
"options": {"device":"fs-123","o":"tls","type":"efs"},
"mounts": {"id1": 1, "id2": 2}
}
}
}`,
pluginAssertions: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
assert.Len(t, plugin.volumes, 1)
vol, ok := plugin.volumes["efsVolume"]
assert.True(t, ok)
assert.Equal(t, "efs", vol.Type)
assert.Equal(t, VolumeMountPathPrefix+"efsVolume", vol.Path)
vols := plugin.state.VolState.Volumes
assert.Len(t, vols, 1)
volInfo, ok := vols["efsVolume"]
require.True(t, ok)
assert.Equal(t, "efs", volInfo.Type)
assert.Equal(t, VolumeMountPathPrefix+"efsVolume", volInfo.Path)
assert.Equal(t, map[string]int{"id1": 1, "id2": 2}, vols["efsVolume"].Mounts)
},
},
volumes: make(map[string]*types.Volume),
state: NewStateManager(),
}
fileExists = func(path string) bool {
return true
}
readStateFile = func() ([]byte, error) {
return []byte(`{"volumes":{"efsVolume":{"type":"efs","path":"/var/lib/ecs/volumes/efsVolume","options":{"device":"fs-123","o":"tls","type":"efs"}}}}`), nil

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
plugin := &AmazonECSVolumePlugin{
volumeDrivers: map[string]driver.VolumeDriver{
"efs": NewECSVolumeDriver(),
},
volumes: make(map[string]*types.Volume),
state: NewStateManager(),
}
fileExists = func(path string) bool {
return true
}
readStateFile = func() ([]byte, error) {
return []byte(tc.stateFileContents), nil
}
defer func() {
fileExists = checkFile
readStateFile = readFile
}()
assert.NoError(t, plugin.LoadState(), "expected no error when loading state")
tc.pluginAssertions(t, plugin)
})
}
defer func() {
fileExists = checkFile
readStateFile = readFile
}()
assert.NoError(t, plugin.LoadState(), "expected no error when loading state")
assert.Len(t, plugin.volumes, 1)
vol, ok := plugin.volumes["efsVolume"]
assert.True(t, ok)
assert.Equal(t, "efs", vol.Type)
assert.Equal(t, VolumeMountPathPrefix+"efsVolume", vol.Path)
vols := plugin.state.VolState.Volumes
assert.Len(t, vols, 1)
volInfo, ok := vols["efsVolume"]
assert.True(t, ok)
assert.Equal(t, "efs", volInfo.Type)
assert.Equal(t, VolumeMountPathPrefix+"efsVolume", volInfo.Path)
}

func TestPluginNoStateFile(t *testing.T) {
Expand Down Expand Up @@ -758,7 +819,7 @@ func TestPluginMount(t *testing.T) {
req: &volume.MountRequest{Name: volName, ID: reqMountID},
expectedResponse: &volume.MountResponse{Mountpoint: volPath},
assertPluginState: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
mounts := map[string]*string{reqMountID: nil}
mounts := map[string]int{reqMountID: 1}
assert.Equal(t,
map[string]*types.Volume{
volName: {Path: volPath, Options: volOpts, Mounts: mounts},
Expand All @@ -778,13 +839,13 @@ func TestPluginMount(t *testing.T) {
pluginVolumes: map[string]*types.Volume{
volName: {
Path: volPath,
Mounts: map[string]*string{"someMount": nil},
Mounts: map[string]int{"someMount": 1},
},
},
req: &volume.MountRequest{Name: volName, ID: reqMountID},
expectedResponse: &volume.MountResponse{Mountpoint: volPath},
assertPluginState: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
mounts := map[string]*string{reqMountID: nil, "someMount": nil}
mounts := map[string]int{reqMountID: 1, "someMount": 1}
assert.Equal(t,
map[string]*types.Volume{volName: {Path: volPath, Mounts: mounts}},
plugin.volumes)
Expand Down Expand Up @@ -826,24 +887,24 @@ func TestPluginMount(t *testing.T) {
assert.Equal(t, map[string]*types.Volume{volName: {Path: volPath}}, plugin.volumes)
assert.Equal(t,
&VolumeState{Volumes: map[string]*VolumeInfo{
volName: {Path: volPath, Mounts: map[string]*string{}},
volName: {Path: volPath, Mounts: map[string]int{}},
}},
plugin.state.VolState)
},
},
{
name: "duplicate mount is a no-op",
name: "duplicate mount increments mount reference count",
pluginVolumes: map[string]*types.Volume{
volName: {
Path: volPath,
Mounts: map[string]*string{reqMountID: nil},
Mounts: map[string]int{reqMountID: 1},
Options: volOpts,
},
},
req: &volume.MountRequest{Name: volName, ID: reqMountID},
expectedResponse: &volume.MountResponse{Mountpoint: volPath},
assertPluginState: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
mounts := map[string]*string{reqMountID: nil}
mounts := map[string]int{reqMountID: 2}
assert.Equal(t,
map[string]*types.Volume{
volName: {Path: volPath, Options: volOpts, Mounts: mounts},
Expand All @@ -870,7 +931,7 @@ func TestPluginMount(t *testing.T) {
expectedError: "mount failed due to an error while saving state: some error",
assertPluginState: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
// No mounts expected on the volume
mounts := map[string]*string{}
mounts := map[string]int{}
assert.Equal(t,
map[string]*types.Volume{volName: {Path: volPath, Mounts: mounts}},
plugin.volumes)
Expand Down Expand Up @@ -963,11 +1024,11 @@ func TestPluginUnmount(t *testing.T) {
d.EXPECT().Remove(&driver.RemoveRequest{Name: volName}).Return(nil)
},
pluginVolumes: map[string]*types.Volume{
volName: {Path: volPath, Mounts: map[string]*string{reqMountID: nil}},
volName: {Path: volPath, Mounts: map[string]int{reqMountID: 1}},
},
req: &volume.UnmountRequest{Name: volName, ID: reqMountID},
assertPluginState: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
mounts := map[string]*string{}
mounts := map[string]int{}
assert.Equal(t,
map[string]*types.Volume{volName: {Path: volPath, Mounts: mounts}},
plugin.volumes)
Expand All @@ -983,12 +1044,33 @@ func TestPluginUnmount(t *testing.T) {
pluginVolumes: map[string]*types.Volume{
volName: {
Path: volPath,
Mounts: map[string]*string{"someMount": nil, reqMountID: nil},
Mounts: map[string]int{"someMount": 1, reqMountID: 1},
},
},
req: &volume.UnmountRequest{Name: volName, ID: reqMountID},
assertPluginState: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
mounts := map[string]int{"someMount": 1}
assert.Equal(t,
map[string]*types.Volume{volName: {Path: volPath, Mounts: mounts}},
plugin.volumes)
assert.Equal(t,
&VolumeState{
Volumes: map[string]*VolumeInfo{volName: {Path: volPath, Mounts: mounts}},
},
plugin.state.VolState)
},
},
{
name: "mount reference count decrements",
pluginVolumes: map[string]*types.Volume{
volName: {
Path: volPath,
Mounts: map[string]int{reqMountID: 2},
},
},
req: &volume.UnmountRequest{Name: volName, ID: reqMountID},
assertPluginState: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
mounts := map[string]*string{"someMount": nil}
mounts := map[string]int{reqMountID: 1}
assert.Equal(t,
map[string]*types.Volume{volName: {Path: volPath, Mounts: mounts}},
plugin.volumes)
Expand Down Expand Up @@ -1023,13 +1105,13 @@ func TestPluginUnmount(t *testing.T) {
Return(errors.New("some error"))
},
pluginVolumes: map[string]*types.Volume{
volName: {Path: volPath, Mounts: map[string]*string{reqMountID: nil}},
volName: {Path: volPath, Mounts: map[string]int{reqMountID: 1}},
},
req: &volume.UnmountRequest{Name: volName, ID: reqMountID},
expectedError: "failed to unmount volume volume: some error",
assertPluginState: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
// Mount should not exist in the plugin state
mounts := map[string]*string{}
mounts := map[string]int{}
assert.Equal(t,
map[string]*types.Volume{volName: {Path: volPath, Mounts: mounts}},
plugin.volumes)
Expand All @@ -1040,7 +1122,7 @@ func TestPluginUnmount(t *testing.T) {
pluginVolumes: map[string]*types.Volume{volName: {Path: volPath, Options: volOpts}},
req: &volume.UnmountRequest{Name: volName, ID: reqMountID},
assertPluginState: func(t *testing.T, plugin *AmazonECSVolumePlugin) {
mounts := map[string]*string{}
mounts := map[string]int{}
assert.Equal(t,
map[string]*types.Volume{
volName: {Path: volPath, Mounts: nil, Options: volOpts},
Expand Down
12 changes: 6 additions & 6 deletions ecs-init/volumes/state_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ type VolumeState struct {

// VolumeInfo contains the information of managed volumes
type VolumeInfo struct {
Type string `json:"type,omitempty"`
Path string `json:"path,omitempty"`
Options map[string]string `json:"options,omitempty"`
CreatedAt string `json:"createdAt,omitempty"`
Mounts map[string]*string `json:"mounts,omitempty"`
Type string `json:"type,omitempty"`
Path string `json:"path,omitempty"`
Options map[string]string `json:"options,omitempty"`
CreatedAt string `json:"createdAt,omitempty"`
Mounts map[string]int `json:"mounts,omitempty"`
}

// NewStateManager initializes the state manager of volume plugin
Expand All @@ -65,7 +65,7 @@ func NewStateManager() *StateManager {

func (s *StateManager) recordVolume(volName string, vol *types.Volume) error {
// Copy the mounts so that the map is not shared
mountsCopy := map[string]*string{}
mountsCopy := map[string]int{}
for k, v := range vol.Mounts {
mountsCopy[k] = v
}
Expand Down
29 changes: 29 additions & 0 deletions ecs-init/volumes/state_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestSaveStateSuccess(t *testing.T) {
Expand Down Expand Up @@ -85,6 +86,34 @@ func TestLoadStateSuccess(t *testing.T) {
assert.NoError(t, s.load(oldState))
}

// Tests backwards compatibility of state loading.
// A change to state format has been introduced with reference counting of volume mounts.
func TestLoadStateFromOldFormat(t *testing.T) {
s := NewStateManager()
oldState := &VolumeState{}
readStateFile = func() ([]byte, error) {
return []byte(`
{
"volumes": {
"efsVolume": {
"type":"efs",
"mounts": {"id1": null}
}
}
}`), nil
}
defer func() {
readStateFile = readFile
}()
require.NoError(t, s.load(oldState))
assert.Equal(t,
&VolumeState{Volumes: map[string]*VolumeInfo{"efsVolume": {
Type: "efs",
Mounts: map[string]int{"id1": 0},
}}},
oldState)
}

func TestLoadInvalidState(t *testing.T) {
s := NewStateManager()
oldState := &VolumeState{}
Expand Down
Loading

0 comments on commit 79aa903

Please sign in to comment.