fingerprint: don't clear Consul/Vault attributes on failure

.changelog/14673.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+fingerprint: Consul and Vault attributes are no longer cleared on fingerprinting failure
+```

client/fingerprint/consul.go

@@ -75,14 +75,6 @@ func (f *ConsulFingerprint) Periodic() (bool, time.Duration) {
 	return true, 15 * time.Second
 }
 
-// clearConsulAttributes removes consul attributes and links from the passed Node.
-func (f *ConsulFingerprint) clearConsulAttributes(r *FingerprintResponse) {
-	for attr := range f.extractors {
-		r.RemoveAttribute(attr)
-	}
-	r.RemoveLink("consul")
-}
-
 func (f *ConsulFingerprint) initialize(req *FingerprintRequest) error {
 	// Only create the Consul client once to avoid creating many connections
 	if f.client == nil {
@@ -118,8 +110,6 @@ func (f *ConsulFingerprint) query(resp *FingerprintResponse) agentconsul.Self {
 	// If we can't hit this URL consul is probably not running on this machine.
 	info, err := f.client.Agent().Self()
 	if err != nil {
-		f.clearConsulAttributes(resp)
-
 		// indicate consul no longer available
 		if f.lastState == consulAvailable {
 			f.logger.Info("consul agent is unavailable")

client/fingerprint/consul_test.go

@@ -411,20 +411,9 @@ func TestConsulFingerprint_Fingerprint_oss(t *testing.T) {
 
 	// execute second query with error
 	err2 := cf.Fingerprint(&FingerprintRequest{Config: cfg, Node: node}, &resp2)
-	require.NoError(t, err2)            // does not return error
-	require.Equal(t, map[string]string{ // attributes set empty
-		"consul.datacenter":    "",
-		"consul.revision":      "",
-		"consul.segment":       "",
-		"consul.server":        "",
-		"consul.sku":           "",
-		"consul.version":       "",
-		"unique.consul.name":   "",
-		"consul.connect":       "",
-		"consul.grpc":          "",
-		"consul.ft.namespaces": "",
-	}, resp2.Attributes)
-	require.True(t, resp.Detected) // never downgrade
+	require.NoError(t, err2)         // does not return error
+	require.Nil(t, resp2.Attributes) // attributes unset so they don't change
+	require.True(t, resp.Detected)   // never downgrade
 
 	// consul no longer available
 	require.Equal(t, consulUnavailable, cf.lastState)
@@ -501,20 +490,9 @@ func TestConsulFingerprint_Fingerprint_ent(t *testing.T) {
 
 	// execute second query with error
 	err2 := cf.Fingerprint(&FingerprintRequest{Config: cfg, Node: node}, &resp2)
-	require.NoError(t, err2)            // does not return error
-	require.Equal(t, map[string]string{ // attributes set empty
-		"consul.datacenter":    "",
-		"consul.revision":      "",
-		"consul.segment":       "",
-		"consul.server":        "",
-		"consul.sku":           "",
-		"consul.version":       "",
-		"consul.ft.namespaces": "",
-		"consul.connect":       "",
-		"consul.grpc":          "",
-		"unique.consul.name":   "",
-	}, resp2.Attributes)
-	require.True(t, resp.Detected) // never downgrade
+	require.NoError(t, err2)         // does not return error
+	require.Nil(t, resp2.Attributes) // attributes unset so they don't change
+	require.True(t, resp.Detected)   // never downgrade
 
 	// consul no longer available
 	require.Equal(t, consulUnavailable, cf.lastState)

client/fingerprint/vault.go

@@ -51,7 +51,7 @@ func (f *VaultFingerprint) Fingerprint(req *FingerprintRequest, resp *Fingerprin
 	// Connect to vault and parse its information
 	status, err := f.client.Sys().SealStatus()
 	if err != nil {
-		f.clearVaultAttributes(resp)
+
 		// Print a message indicating that Vault is not available anymore
 		if f.lastState == vaultAvailable {
 			f.logger.Info("Vault is unavailable")
@@ -80,10 +80,3 @@ func (f *VaultFingerprint) Fingerprint(req *FingerprintRequest, resp *Fingerprin
 func (f *VaultFingerprint) Periodic() (bool, time.Duration) {
 	return true, 15 * time.Second
 }
-
-func (f *VaultFingerprint) clearVaultAttributes(r *FingerprintResponse) {
-	r.RemoveAttribute("vault.accessible")
-	r.RemoveAttribute("vault.version")
-	r.RemoveAttribute("vault.cluster_id")
-	r.RemoveAttribute("vault.cluster_name")
-}

client/fingerprint/vault_test.go

@@ -39,4 +39,21 @@ func TestVaultFingerprint(t *testing.T) {
 	assertNodeAttributeContains(t, response.Attributes, "vault.version")
 	assertNodeAttributeContains(t, response.Attributes, "vault.cluster_id")
 	assertNodeAttributeContains(t, response.Attributes, "vault.cluster_name")
+
+	tv.Stop()
+
+	err = fp.Fingerprint(request, &response)
+	if err != nil {
+		t.Fatalf("Failed to fingerprint: %s", err)
+	}
+
+	if !response.Detected {
+		t.Fatalf("should still show as detected")
+	}
+
+	assertNodeAttributeContains(t, response.Attributes, "vault.accessible")
+	assertNodeAttributeContains(t, response.Attributes, "vault.version")
+	assertNodeAttributeContains(t, response.Attributes, "vault.cluster_id")
+	assertNodeAttributeContains(t, response.Attributes, "vault.cluster_name")
+
 }

testutil/vault.go

@@ -1,6 +1,7 @@
 package testutil
 
 import (
+	"errors"
 	"fmt"
 	"math/rand"
 	"os"
@@ -215,6 +216,9 @@ func (tv *TestVault) Stop() {
 	}
 
 	if err := tv.cmd.Process.Kill(); err != nil {
+		if errors.Is(err, os.ErrProcessDone) {
+			return
+		}
 		tv.t.Errorf("err: %s", err)
 	}
 	if tv.waitCh != nil {