Merge pull request #8919 from dokku/8918-renaming-or-cloning-an-app-silently-stops-vector-log-shipping

Regenerate vector config on app lifecycle changes

plugins/logs/functions.go

@@ -538,6 +538,17 @@ func vectorScopes() []vectorAppSinks {
 	})
 }
 
+// regenerateVectorConfig rewrites the generated vector config so that it matches
+// the current set of apps and their properties. App lifecycle triggers call this
+// instead of writeVectorConfig because the config is derived state: failing to
+// rewrite it should not abort the app operation that changed the state it is
+// derived from.
+func regenerateVectorConfig() {
+	if err := writeVectorConfig(); err != nil {
+		common.LogWarn(fmt.Sprintf("Unable to write updated vector config: %s", err.Error()))
+	}
+}
+
 func writeVectorConfig() error {
 	data, err := buildVectorConfig(vectorScopes())
 	if err != nil {

plugins/logs/functions_test.go

@@ -2,6 +2,7 @@ package logs
 
 import (
 	"encoding/json"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -291,6 +292,10 @@ func setupScopesTest(t *testing.T, apps []string) {
 		t.Fatalf("PropertySetup: %v", err)
 	}
 
+	if err := common.CreateDataDirectory("logs"); err != nil {
+		t.Fatalf("CreateDataDirectory: %v", err)
+	}
+
 	for _, appName := range apps {
 		if err := os.MkdirAll(filepath.Join(os.Getenv("DOKKU_ROOT"), appName), 0755); err != nil {
 			t.Fatalf("MkdirAll %s: %v", appName, err)
@@ -298,6 +303,141 @@ func setupScopesTest(t *testing.T, apps []string) {
 	}
 }
 
+// setLogsProperty sets a logs property for the given scope and regenerates the
+// config, standing in for the logs:set command
+func setLogsProperty(t *testing.T, appName string, property string, value string) {
+	t.Helper()
+
+	if err := common.PropertyWrite("logs", appName, property, value); err != nil {
+		t.Fatalf("PropertyWrite %s %s: %v", appName, property, err)
+	}
+
+	if err := writeVectorConfig(); err != nil {
+		t.Fatalf("writeVectorConfig: %v", err)
+	}
+}
+
+// readVectorConfig decodes the generated config from disk
+func readVectorConfig(t *testing.T) map[string]interface{} {
+	t.Helper()
+
+	b, err := os.ReadFile(filepath.Join(common.GetDataDirectory("logs"), "vector.json"))
+	if err != nil {
+		t.Fatalf("ReadFile vector.json: %v", err)
+	}
+
+	var decoded map[string]interface{}
+	if err := json.Unmarshal(b, &decoded); err != nil {
+		t.Fatalf("Unmarshal vector.json: %v", err)
+	}
+
+	return decoded
+}
+
+// assertComponents checks whether an app has a source and a sink in the config
+func assertComponents(t *testing.T, decoded map[string]interface{}, appName string, want bool) {
+	t.Helper()
+
+	components := map[string]string{
+		"sources": fmt.Sprintf("docker-source:%s", appName),
+		"sinks":   fmt.Sprintf("docker-sink:%s", appName),
+	}
+
+	for section, id := range components {
+		group, ok := lookup(t, decoded, section).(map[string]interface{})
+		if !ok {
+			t.Fatalf("%s is not a map", section)
+		}
+
+		if _, ok := group[id]; ok != want {
+			t.Errorf("%s[%q] exists = %v, want %v", section, id, ok, want)
+		}
+	}
+}
+
+// TestTriggerPostDeleteRewritesVectorConfig covers a destroyed app leaving its
+// source and sink behind forever. post-delete fires before the app root is
+// removed, so the app is still listed by UnfilteredDokkuApps at this point and
+// only drops out because its properties are destroyed first.
+func TestTriggerPostDeleteRewritesVectorConfig(t *testing.T) {
+	setupScopesTest(t, []string{"appa", "appb"})
+	setLogsProperty(t, "appa", "vector-sink", "console://?encoding[codec]=json")
+	setLogsProperty(t, "appb", "vector-sink", "console://?encoding[codec]=json")
+
+	assertComponents(t, readVectorConfig(t), "appa", true)
+
+	if err := TriggerPostDelete("appa"); err != nil {
+		t.Fatalf("TriggerPostDelete: %v", err)
+	}
+
+	decoded := readVectorConfig(t)
+	assertComponents(t, decoded, "appa", false)
+	assertComponents(t, decoded, "appb", true)
+}
+
+// TestTriggerPostAppRenameSetupRewritesVectorConfig covers the rename case: the
+// sink property follows the app, so without a regeneration the surviving source
+// filters on a label no container carries and the new name has no source at all.
+func TestTriggerPostAppRenameSetupRewritesVectorConfig(t *testing.T) {
+	setupScopesTest(t, []string{"appa", "appa-renamed"})
+	setLogsProperty(t, "appa", "vector-sink", "console://?encoding[codec]=json")
+
+	if err := TriggerPostAppRenameSetup("appa", "appa-renamed"); err != nil {
+		t.Fatalf("TriggerPostAppRenameSetup: %v", err)
+	}
+
+	decoded := readVectorConfig(t)
+	assertComponents(t, decoded, "appa", false)
+	assertComponents(t, decoded, "appa-renamed", true)
+
+	labels := lookup(t, decoded, "sources", "docker-source:appa-renamed", "include_labels")
+	if got := labels.([]interface{})[0]; got != "com.dokku.app-name=appa-renamed" {
+		t.Errorf("include_labels[0] = %v, want com.dokku.app-name=appa-renamed", got)
+	}
+}
+
+// TestTriggerPostAppCloneSetupRewritesVectorConfig covers the clone case, where
+// the clone inherits a sink but would otherwise have no source feeding it.
+func TestTriggerPostAppCloneSetupRewritesVectorConfig(t *testing.T) {
+	setupScopesTest(t, []string{"appa", "appa-clone"})
+	setLogsProperty(t, "appa", "vector-sink", "console://?encoding[codec]=json")
+
+	if err := TriggerPostAppCloneSetup("appa", "appa-clone"); err != nil {
+		t.Fatalf("TriggerPostAppCloneSetup: %v", err)
+	}
+
+	decoded := readVectorConfig(t)
+	assertComponents(t, decoded, "appa", true)
+	assertComponents(t, decoded, "appa-clone", true)
+}
+
+// TestTriggerPostAppRenameSetupRewritesGlobalRelabel pins the generated VRL,
+// which embeds app names directly as branches of the global relabel transform.
+// A rename leaves a branch naming an app that no longer exists, so the renamed
+// app ships under the global alias rather than its own.
+func TestTriggerPostAppRenameSetupRewritesGlobalRelabel(t *testing.T) {
+	setupScopesTest(t, []string{"appa", "appa-renamed"})
+	setLogsProperty(t, "--global", "vector-sink", "console://?encoding[codec]=json")
+	setLogsProperty(t, "appa", "app-label-alias", "app_name")
+
+	source := lookup(t, readVectorConfig(t), "transforms", "docker-global-relabel", "source").(string)
+	if !strings.Contains(source, `app == "appa"`) {
+		t.Fatalf("relabel source %q missing the pre-rename branch", source)
+	}
+
+	if err := TriggerPostAppRenameSetup("appa", "appa-renamed"); err != nil {
+		t.Fatalf("TriggerPostAppRenameSetup: %v", err)
+	}
+
+	source = lookup(t, readVectorConfig(t), "transforms", "docker-global-relabel", "source").(string)
+	if strings.Contains(source, `app == "appa"`) {
+		t.Errorf("relabel source %q still names the old app", source)
+	}
+	if !strings.Contains(source, `app == "appa-renamed"`) {
+		t.Errorf("relabel source %q missing the renamed app", source)
+	}
+}
+
 func scopeByID(t *testing.T, scopes []vectorAppSinks, sourceID string) vectorAppSinks {
 	t.Helper()
 

plugins/logs/triggers.go

@@ -106,14 +106,20 @@ func TriggerLogsGetProperty(appName string, key string) error {
 	return nil
 }
 
-// TriggerPostAppCloneSetup creates new logs files
+// TriggerPostAppCloneSetup creates new logs files and regenerates the vector
+// config so that the clone gets a source of its own for the sink it inherited
 func TriggerPostAppCloneSetup(oldAppName string, newAppName string) error {
 	err := common.PropertyClone("logs", oldAppName, newAppName)
 	if err != nil {
 		return err
 	}
 
-	return common.CloneAppData("logs", oldAppName, newAppName)
+	if err := common.CloneAppData("logs", oldAppName, newAppName); err != nil {
+		return err
+	}
+
+	regenerateVectorConfig()
+	return nil
 }
 
 // TriggerPostAppRename removes the old app data
@@ -121,7 +127,9 @@ func TriggerPostAppRename(oldAppName string, newAppName string) error {
 	return common.MigrateAppDataDirectory("logs", oldAppName, newAppName)
 }
 
-// TriggerPostAppRenameSetup renames logs files
+// TriggerPostAppRenameSetup renames logs files and regenerates the vector config.
+// Both app roots exist at this point, but the old app no longer has any logs
+// properties, so it drops out of the generated config.
 func TriggerPostAppRenameSetup(oldAppName string, newAppName string) error {
 	if err := common.PropertyClone("logs", oldAppName, newAppName); err != nil {
 		return err
@@ -131,7 +139,12 @@ func TriggerPostAppRenameSetup(oldAppName string, newAppName string) error {
 		return err
 	}
 
-	return common.CloneAppData("logs", oldAppName, newAppName)
+	if err := common.CloneAppData("logs", oldAppName, newAppName); err != nil {
+		return err
+	}
+
+	regenerateVectorConfig()
+	return nil
 }
 
 // TriggerPostCreate ensures apps have the correct data directory structure
@@ -139,11 +152,16 @@ func TriggerPostCreate(appName string) error {
 	return common.CreateAppDataDirectory("logs", appName)
 }
 
-// TriggerPostDelete destroys the logs property for a given app container
+// TriggerPostDelete destroys the logs property for a given app container and
+// regenerates the vector config. The regeneration runs after the properties are
+// destroyed so that the app drops out of the config even though its app root is
+// only removed later in the destroy flow.
 func TriggerPostDelete(appName string) error {
 	dataErr := common.RemoveAppDataDirectory("logs", appName)
 	propertyErr := common.PropertyDestroy("logs", appName)
 
+	regenerateVectorConfig()
+
 	if dataErr != nil {
 		return dataErr
 	}