From 9d2c34fa4f919cab1cda28e006d5174afcb1b326 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 2 Jan 2019 22:31:27 +0000 Subject: [PATCH 1/2] Allow MGS format to return 17 --- fs-ansible/roles/lustre/tasks/format.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs-ansible/roles/lustre/tasks/format.yaml b/fs-ansible/roles/lustre/tasks/format.yaml index 53c1403a..3aca85d0 100644 --- a/fs-ansible/roles/lustre/tasks/format.yaml +++ b/fs-ansible/roles/lustre/tasks/format.yaml @@ -10,7 +10,7 @@ - name: Ensure MGS has been formatted command: /usr/sbin/mkfs.lustre --mgs /dev/{{ mgs }} register: command_result - failed_when: "command_result.rc != 0 and ('was previously formatted for lustre' not in command_result.stderr)" + failed_when: "command_result.rc != 0 and ('was previously formatted for lustre' not in command_result.stderr) and command_result.rc != 17" changed_when: "command_result.rc == 0" when: - mgs is defined From f18df5ed66275229774d964d558249b1ca7f977f Mon Sep 17 00:00:00 2001 From: John Date: Wed, 2 Jan 2019 23:33:59 +0000 Subject: [PATCH 2/2] Allow umount to be attempted twice --- internal/pkg/pfsprovider/ansible/mount.go | 11 ++++++- .../pkg/pfsprovider/ansible/mount_test.go | 32 ++++++++++--------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/internal/pkg/pfsprovider/ansible/mount.go b/internal/pkg/pfsprovider/ansible/mount.go index 0046b7fd..37e30753 100644 --- a/internal/pkg/pfsprovider/ansible/mount.go +++ b/internal/pkg/pfsprovider/ansible/mount.go @@ -197,7 +197,16 @@ func fixUpOwnership(hostname string, owner uint, group uint, directory string) e } func umountLustre(hostname string, directory string) error { - return runner.Execute(hostname, fmt.Sprintf("umount -l %s", directory)) + // only unmount if already mounted + if err := runner.Execute(hostname, fmt.Sprintf("grep %s /etc/mtab", directory)); err == nil { + if err := runner.Execute(hostname, fmt.Sprintf("umount -l %s", directory)); err != nil { + return err + } + } else { + // TODO: we should really just avoid this being possible? + log.Println("skip umount, as not currently mounted.") + } + return nil } func removeSubtree(hostname string, directory string) error { diff --git a/internal/pkg/pfsprovider/ansible/mount_test.go b/internal/pkg/pfsprovider/ansible/mount_test.go index 879d63ac..0c50a467 100644 --- a/internal/pkg/pfsprovider/ansible/mount_test.go +++ b/internal/pkg/pfsprovider/ansible/mount_test.go @@ -168,12 +168,12 @@ func Test_Umount(t *testing.T) { AttachPrivateNamespace: true, AttachAsSwapBytes: 10000, Attachments: []registry.Attachment{ - {Hostname: "client1", Job: "job1", State: registry.RequestDetach}, - {Hostname: "client2", Job: "job1", State: registry.RequestDetach}, + {Hostname: "client1", Job: "job4", State: registry.RequestDetach}, + {Hostname: "client2", Job: "job4", State: registry.RequestDetach}, {Hostname: "client3", Job: "job3", State: registry.Attached}, {Hostname: "client3", Job: "job3", State: registry.RequestAttach}, {Hostname: "client3", Job: "job3", State: registry.Detached}, - {Hostname: "client2", Job: "job2", State: registry.RequestDetach}, + {Hostname: "client2", Job: "job1", State: registry.RequestDetach}, }, ClientPort: 42, Owner: 1001, @@ -185,21 +185,22 @@ func Test_Umount(t *testing.T) { } err := umount(Lustre, volume, bricks) assert.Nil(t, err) - assert.Equal(t, 18, fake.calls) + assert.Equal(t, 20, fake.calls) assert.Equal(t, "client1", fake.hostnames[0]) assert.Equal(t, "swapoff /dev/loop42", fake.cmdStrs[0]) assert.Equal(t, "losetup -d /dev/loop42", fake.cmdStrs[1]) - assert.Equal(t, "rm -rf /dac/job1_job/swap/client1", fake.cmdStrs[2]) - assert.Equal(t, "rm -rf /dac/job1_job_private", fake.cmdStrs[3]) - assert.Equal(t, "umount -l /dac/job1_job", fake.cmdStrs[4]) - assert.Equal(t, "rm -rf /dac/job1_job", fake.cmdStrs[5]) + assert.Equal(t, "rm -rf /dac/job4_job/swap/client1", fake.cmdStrs[2]) + assert.Equal(t, "rm -rf /dac/job4_job_private", fake.cmdStrs[3]) + assert.Equal(t, "grep /dac/job4_job /etc/mtab", fake.cmdStrs[4]) + assert.Equal(t, "umount -l /dac/job4_job", fake.cmdStrs[5]) + assert.Equal(t, "rm -rf /dac/job4_job", fake.cmdStrs[6]) - assert.Equal(t, "client2", fake.hostnames[6]) - assert.Equal(t, "swapoff /dev/loop42", fake.cmdStrs[6]) + assert.Equal(t, "client2", fake.hostnames[7]) + assert.Equal(t, "swapoff /dev/loop42", fake.cmdStrs[7]) - assert.Equal(t, "client2", fake.hostnames[17]) - assert.Equal(t, "rm -rf /dac/job2_job", fake.cmdStrs[17]) + assert.Equal(t, "client2", fake.hostnames[19]) + assert.Equal(t, "rm -rf /dac/job1_job", fake.cmdStrs[19]) } func Test_Umount_multi(t *testing.T) { @@ -225,11 +226,12 @@ func Test_Umount_multi(t *testing.T) { } err := umount(Lustre, volume, bricks) assert.Nil(t, err) - assert.Equal(t, 2, fake.calls) + assert.Equal(t, 3, fake.calls) assert.Equal(t, "client1", fake.hostnames[0]) - assert.Equal(t, "umount -l /dac/job1_persistent_asdf", fake.cmdStrs[0]) - assert.Equal(t, "rm -rf /dac/job1_persistent_asdf", fake.cmdStrs[1]) + assert.Equal(t, "grep /dac/job1_persistent_asdf /etc/mtab", fake.cmdStrs[0]) + assert.Equal(t, "umount -l /dac/job1_persistent_asdf", fake.cmdStrs[1]) + assert.Equal(t, "rm -rf /dac/job1_persistent_asdf", fake.cmdStrs[2]) } func Test_Mount_multi(t *testing.T) {