Revert Proxy Lifecycle PRs (#152)

* Revert "proxy-lifecycle: catch SIGTERM and initiate graceful shutdown (#130)"

This reverts commit 40c99dc.

* Revert "proxy-lifecycle: add HTTP Server with endpoints for proxy lifecycle shutdown (#115)"

This reverts commit 0047e65.

* Revert "cmd: add CLI flags for proxy shutdown lifecycle management (#100)"

This reverts commit 3d37b9f.

.github/workflows/consul-dataplane-checks.yaml

@@ -32,7 +32,7 @@ jobs:
       - uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0
         with:
           go-version: ${{ needs.get-go-version.outputs.go-version }}
-      - run: go test ./... -p 1 # disable parallelism to avoid port conflicts from default metrics and lifecycle server configuration
+      - run: go test ./...
   integration-tests:
     name: integration-tests
     needs:

Makefile

@@ -110,7 +110,7 @@ else
 	$(error Cannot generate changelog without LAST_RELEASE_GIT_TAG)
 endif
 
-INTEGRATION_TESTS_SERVER_IMAGE    ?= hashicorppreview/consul:1.15-dev
+INTEGRATION_TESTS_SERVER_IMAGE    ?= hashicorppreview/consul:1.14-dev
 INTEGRATION_TESTS_DATAPLANE_IMAGE ?= $(PRODUCT_NAME)/release-default:$(VERSION)
 
 .PHONY: expand-integration-tests-output-dir

cmd/consul-dataplane/main.go

@@ -62,26 +62,17 @@ var (
 	promScrapePath        string
 	promMergePort         int
 
-	adminBindAddr         string
-	adminBindPort         int
-	readyBindAddr         string
-	readyBindPort         int
-	envoyConcurrency      int
-	envoyDrainTimeSeconds int
-	envoyDrainStrategy    string
+	adminBindAddr    string
+	adminBindPort    int
+	readyBindAddr    string
+	readyBindPort    int
+	envoyConcurrency int
 
 	xdsBindAddr string
 	xdsBindPort int
 
 	consulDNSBindAddr string
 	consulDNSPort     int
-
-	shutdownDrainListenersEnabled bool
-	shutdownGracePeriodSeconds    int
-	gracefulShutdownPath          string
-	gracefulPort                  int
-
-	dumpEnvoyConfigOnExitEnabled bool
 )
 
 func init() {
@@ -135,8 +126,6 @@ func init() {
 	StringVar(&readyBindAddr, "envoy-ready-bind-address", "", "DP_ENVOY_READY_BIND_ADDRESS", "The address on which Envoy's readiness probe is available.")
 	IntVar(&readyBindPort, "envoy-ready-bind-port", 0, "DP_ENVOY_READY_BIND_PORT", "The port on which Envoy's readiness probe is available.")
 	IntVar(&envoyConcurrency, "envoy-concurrency", 2, "DP_ENVOY_CONCURRENCY", "The number of worker threads that Envoy uses.")
-	IntVar(&envoyDrainTimeSeconds, "envoy-drain-time-seconds", 30, "DP_ENVOY_DRAIN_TIME", "The time in seconds for which Envoy will drain connections.")
-	StringVar(&envoyDrainStrategy, "envoy-drain-strategy", "immediate", "DP_ENVOY_DRAIN_STRATEGY", "The behaviour of Envoy during the drain sequence. Determines whether all open connections should be encouraged to drain immediately or to increase the percentage gradually as the drain time elapses.")
 
 	StringVar(&xdsBindAddr, "xds-bind-addr", "127.0.0.1", "DP_XDS_BIND_ADDR", "The address on which the Envoy xDS server is available.")
 	IntVar(&xdsBindPort, "xds-bind-port", 0, "DP_XDS_BIND_PORT", "The port on which the Envoy xDS server is available.")
@@ -150,18 +139,6 @@ func init() {
 
 	StringVar(&consulDNSBindAddr, "consul-dns-bind-addr", "127.0.0.1", "DP_CONSUL_DNS_BIND_ADDR", "The address that will be bound to the consul dns proxy.")
 	IntVar(&consulDNSPort, "consul-dns-bind-port", -1, "DP_CONSUL_DNS_BIND_PORT", "The port the consul dns proxy will listen on. By default -1 disables the dns proxy")
-
-	// Default is false because it will generally be configured appropriately by Helm
-	// configuration or pod annotation.
-	BoolVar(&shutdownDrainListenersEnabled, "shutdown-drain-listeners", false, "DP_SHUTDOWN_DRAIN_LISTENERS", "Wait for proxy listeners to drain before terminating the proxy container.")
-	// Default is 0 because it will generally be configured appropriately by Helm
-	// configuration or pod annotation.
-	IntVar(&shutdownGracePeriodSeconds, "shutdown-grace-period-seconds", 0, "DP_SHUTDOWN_GRACE_PERIOD_SECONDS", "Amount of time to wait after receiving a SIGTERM signal before terminating the proxy.")
-	StringVar(&gracefulShutdownPath, "graceful-shutdown-path", "/graceful_shutdown", "DP_GRACEFUL_SHUTDOWN_PATH", "An HTTP path to serve the graceful shutdown endpoint.")
-	IntVar(&gracefulPort, "graceful-port", 20300, "DP_GRACEFUL_PORT", "A port to serve HTTP endpoints for graceful shutdown.")
-
-	// Default is false, may be useful for debugging unexpected termination.
-	BoolVar(&dumpEnvoyConfigOnExitEnabled, "dump-envoy-config-on-exit", false, "DP_DUMP_ENVOY_CONFIG_ON_EXIT", "Call the Envoy /config_dump endpoint during consul-dataplane controlled shutdown.")
 }
 
 // validateFlags performs semantic validation of the flag values
@@ -173,13 +150,13 @@ func validateFlags() {
 	}
 }
 
-func run() error {
+func main() {
 	flag.Parse()
 
 	if printVersion {
 		fmt.Printf("Consul Dataplane v%s\n", version.GetHumanVersion())
 		fmt.Printf("Revision %s\n", version.GitCommit)
-		return nil
+		return
 	}
 
 	readServiceIDFromFile()
@@ -239,19 +216,12 @@ func run() error {
 			},
 		},
 		Envoy: &consuldp.EnvoyConfig{
-			AdminBindAddress:              adminBindAddr,
-			AdminBindPort:                 adminBindPort,
-			ReadyBindAddress:              readyBindAddr,
-			ReadyBindPort:                 readyBindPort,
-			EnvoyConcurrency:              envoyConcurrency,
-			EnvoyDrainTimeSeconds:         envoyDrainTimeSeconds,
-			EnvoyDrainStrategy:            envoyDrainStrategy,
-			ShutdownDrainListenersEnabled: shutdownDrainListenersEnabled,
-			ShutdownGracePeriodSeconds:    shutdownGracePeriodSeconds,
-			GracefulShutdownPath:          gracefulShutdownPath,
-			GracefulPort:                  gracefulPort,
-			DumpEnvoyConfigOnExitEnabled:  dumpEnvoyConfigOnExitEnabled,
-			ExtraArgs:                     flag.Args(),
+			AdminBindAddress: adminBindAddr,
+			AdminBindPort:    adminBindPort,
+			ReadyBindAddress: readyBindAddr,
+			ReadyBindPort:    readyBindPort,
+			EnvoyConcurrency: envoyConcurrency,
+			ExtraArgs:        flag.Args(),
 		},
 		XDSServer: &consuldp.XDSServer{
 			BindAddress: xdsBindAddr,
@@ -265,28 +235,22 @@ func run() error {
 
 	consuldpInstance, err := consuldp.NewConsulDP(consuldpCfg)
 	if err != nil {
-		return err
+		log.Fatal(err)
 	}
 
 	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
 
 	sigCh := make(chan os.Signal, 1)
 	signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)
 
 	go func() {
-		// Block waiting for SIGTERM
 		<-sigCh
-
-		consuldpInstance.GracefulShutdown(cancel)
+		cancel()
 	}()
 
-	return consuldpInstance.Run(ctx)
-}
-
-func main() {
-	err := run()
+	err = consuldpInstance.Run(ctx)
 	if err != nil {
+		cancel()
 		log.Fatal(err)
 	}
 }

integration-tests/helpers/dataplane.go

@@ -5,6 +5,7 @@ package helpers
 
 import (
 	"fmt"
+	"io"
 	"testing"
 
 	"github.com/testcontainers/testcontainers-go"
@@ -15,16 +16,13 @@ import (
 var EnvoyAdminPort = TCP(30000)
 
 type DataplaneConfig struct {
-	Addresses                     string
-	ServiceNodeName               string
-	ProxyServiceID                string
-	LoginAuthMethod               string
-	LoginBearerToken              string
-	DNSBindPort                   string
-	ServiceMetricsURL             string
-	ShutdownGracePeriodSeconds    string
-	ShutdownDrainListenersEnabled bool
-	DumpEnvoyConfigOnExitEnabled  bool
+	Addresses         string
+	ServiceNodeName   string
+	ProxyServiceID    string
+	LoginAuthMethod   string
+	LoginBearerToken  string
+	DNSBindPort       string
+	ServiceMetricsURL string
 }
 
 func (cfg DataplaneConfig) ToArgs() []string {
@@ -45,19 +43,6 @@ func (cfg DataplaneConfig) ToArgs() []string {
 		"-telemetry-prom-scrape-path", "/metrics",
 		"-telemetry-prom-service-metrics-url", cfg.ServiceMetricsURL,
 	}
-
-	if cfg.ShutdownGracePeriodSeconds != "" {
-		args = append(args, "-shutdown-grace-period-seconds", cfg.ShutdownGracePeriodSeconds)
-	}
-
-	if cfg.ShutdownDrainListenersEnabled {
-		args = append(args, "-shutdown-drain-listeners")
-	}
-
-	if cfg.DumpEnvoyConfigOnExitEnabled {
-		args = append(args, "-dump-envoy-config-on-exit")
-	}
-
 	return args
 }
 
@@ -78,5 +63,30 @@ func RunDataplane(t *testing.T, pod *Pod, suite *Suite, cfg DataplaneConfig) *Co
 		WaitingFor: wait.ForLog("starting main dispatch loop"), // https://github.com/envoyproxy/envoy/blob/ce49966ecb5f2d530117a29ae60b88198746fd74/source/server/server.cc#L906-L907
 	})
 
+	t.Cleanup(func() {
+		url := fmt.Sprintf(
+			"http://%s:%d/config_dump?include_eds",
+			pod.HostIP,
+			pod.MappedPorts[EnvoyAdminPort],
+		)
+
+		rsp, err := httpClient.Get(url)
+		if err != nil {
+			t.Logf("failed to dump Envoy config: %v\n", err)
+			return
+		}
+		defer rsp.Body.Close()
+
+		config, err := io.ReadAll(rsp.Body)
+		if err != nil {
+			t.Logf("failed to dump Envoy config: %v\n", err)
+			return
+		}
+		suite.CaptureArtifact(
+			fmt.Sprintf("%s-envoy-config.json", cfg.ProxyServiceID),
+			config,
+		)
+	})
+
 	return container
 }

integration-tests/helpers/suite.go

@@ -204,17 +204,11 @@ func (s *Suite) Volume(t *testing.T) *Volume {
 		require.NoError(t, err)
 
 		t.Cleanup(func() {
-			s.mu.Lock()
-			defer s.mu.Unlock()
+			ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+			defer cancel()
 
-			if s.volume != nil {
-				ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-				defer cancel()
-
-				if err := docker.VolumeRemove(ctx, v.Name, true); err != nil {
-					t.Logf("failed to remove volume: %v", err)
-				}
-				s.volume = nil
+			if err := docker.VolumeRemove(ctx, v.Name, true); err != nil {
+				t.Logf("failed to remove volume: %v", err)
 			}
 		})