command/clistate: Return an error on unlock failure (#25729)

* Return an error on unlock failure

When the lock can't be released return the err even if there is no previous error with the current action. This allows faster failure in CI/CD systems. Without this failure to remove the lock would result in the failure happening on a subsequent plan or apply which slows down the feedback loop in automated systems.

* Update command/clistate/state.go

Accept review suggestion

Co-authored-by: ZymoticB <ZymoticB@users.noreply.github.com>

* add test

Co-authored-by: ZymoticB <ZymoticB@users.noreply.github.com>
Co-authored-by: Kristin Laemmert <mildwonkey@users.noreply.github.com>

command/clistate/state.go

@@ -149,9 +149,7 @@ func (l *locker) Unlock(parentErr error) error {
 		l.ui.Output(l.color.Color(fmt.Sprintf(
 			"\n"+strings.TrimSpace(UnlockErrorMessage)+"\n", err)))
 
-		if parentErr != nil {
-			parentErr = multierror.Append(parentErr, err)
-		}
+		parentErr = multierror.Append(parentErr, err)
 	}
 
 	return parentErr

command/clistate/state_test.go

@@ -0,0 +1,25 @@
+package clistate
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/hashicorp/terraform/states/statemgr"
+	"github.com/mitchellh/cli"
+	"github.com/mitchellh/colorstring"
+)
+
+func TestUnlock(t *testing.T) {
+	ui := new(cli.MockUi)
+
+	l := NewLocker(context.Background(), 0, ui, &colorstring.Colorize{Disable: true})
+	l.Lock(statemgr.NewUnlockErrorFull(nil, nil), "test-lock")
+
+	err := l.Unlock(nil)
+	if err != nil {
+		fmt.Printf(err.Error())
+	} else {
+		t.Error("expected error")
+	}
+}