Skip to content

Commit

Permalink
Trident should check for ACP only if it is enabled
Browse files Browse the repository at this point in the history
The purpose of this change is to ensure Trident checks for ACP only if ACP is enabled. This prevents starting a go routine that may never succeed when ACP is not enabled. Also, changing some of the warnings to debug in those scenarios.
  • Loading branch information
rohit-arora-dev authored Dec 12, 2023
1 parent e2ac278 commit c921c8b
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 16 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

[Releases](https://github.com/NetApp/trident/releases)

## Changes since v23.07.0
## Changes since v23.10.0

**Fixes:**

- Fixed ACP warning messages when ACP is not enabled. (Issue [#866](https://github.com/NetApp/trident/issues/866)).

## v23.07.0

**Fixes:**

Expand Down
8 changes: 4 additions & 4 deletions acp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ func (c *client) GetVersion(ctx context.Context) (*version.Version, error) {
Logc(ctx).Debug("Getting Trident-ACP version.")

if !c.acpEnabled {
Logc(ctx).Warning("ACP is not enabled.")
return nil, nil
Logc(ctx).Debug("ACP is not enabled.")
return nil, errors.UnsupportedError("ACP is not enabled")
}

acpVersion, err := c.restClient.GetVersion(ctx)
Expand All @@ -84,8 +84,8 @@ func (c *client) GetVersionWithBackoff(ctx context.Context) (*version.Version, e
var err error

if !c.acpEnabled {
Logc(ctx).Warning("ACP is not enabled.")
return nil, nil
Logc(ctx).Debug("ACP is not enabled.")
return nil, errors.UnsupportedError("ACP is not enabled")
}

getVersion := func() error {
Expand Down
2 changes: 1 addition & 1 deletion acp/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestTridentACP_GetVersionWithBackoff(t *testing.T) {

client := newClient(nil, false)
v, err := client.GetVersionWithBackoff(ctx)
assert.Nil(t, err, "unexpected error")
assert.True(t, errors.IsUnsupportedError(err), "unexpected error")
assert.Nil(t, v, "expected nil version")
})

Expand Down
4 changes: 3 additions & 1 deletion frontend/rest/controller_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,9 @@ func GetVersion(w http.ResponseWriter, r *http.Request) {
func GetACPVersion(ctx context.Context) string {
version, err := acp.API().GetVersion(ctx)
if err != nil {
Logc(ctx).WithError(err).Error("Could not get Trident-ACP version.")
if !errors.IsUnsupportedError(err) {
Logc(ctx).WithError(err).Error("Could not get Trident-ACP version.")
}
return ""
}

Expand Down
22 changes: 13 additions & 9 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,15 +517,19 @@ func main() {
acp.Initialize(*acpAddress, *enableACP, config.HTTPTimeout)
Log().Info("Created Trident-ACP REST API Client.")

// Asynchronously check if the Trident-ACP API is available.
go func() {
version, err := acp.API().GetVersionWithBackoff(ctx)
if err != nil || version == nil {
Log().Warning("Failed to get version from Trident-ACP REST API; premium workflows may fail.")
} else {
Log().WithField("version", version.String()).Info("Discovered Trident-ACP Version.")
}
}()
if *enableACP {
// Asynchronously check if the Trident-ACP API is available.
go func() {
version, err := acp.API().GetVersionWithBackoff(ctx)
if err != nil || version == nil {
if !errors.IsUnsupportedError(err) {
Log().Warning("Failed to get version from Trident-ACP REST API; premium workflows may fail.")
}
} else {
Log().WithField("version", version.String()).Info("Discovered Trident-ACP Version.")
}
}()
}

// Bootstrap the orchestrator and start its frontends. Some frontends, notably REST and Docker, must
// start before the core so that the external interfaces are minimally responding while the core is
Expand Down

0 comments on commit c921c8b

Please sign in to comment.