diff --git a/depot/containerstore/containerstore.go b/depot/containerstore/containerstore.go index 53ebcb3d..1a232cfa 100644 --- a/depot/containerstore/containerstore.go +++ b/depot/containerstore/containerstore.go @@ -75,7 +75,6 @@ type containerStore struct { rootFSSizer configuration.RootFSSizer logManager LogManager - useDeclarativeHealthCheck bool declarativeHealthcheckPath string proxyConfigHandler ProxyManager @@ -106,7 +105,6 @@ func New( trustedSystemCertificatesPath string, metronClient loggingclient.IngressClient, rootFSSizer configuration.RootFSSizer, - useDeclarativeHealthCheck bool, declarativeHealthcheckPath string, proxyConfigHandler ProxyManager, cellID string, @@ -129,7 +127,6 @@ func New( metronClient: metronClient, rootFSSizer: rootFSSizer, trustedSystemCertificatesPath: trustedSystemCertificatesPath, - useDeclarativeHealthCheck: useDeclarativeHealthCheck, declarativeHealthcheckPath: declarativeHealthcheckPath, proxyConfigHandler: proxyConfigHandler, @@ -155,7 +152,6 @@ func (cs *containerStore) Reserve(logger lager.Logger, traceID string, req *exec err := cs.containers.Add( newStoreNode(&cs.containerConfig, - cs.useDeclarativeHealthCheck, cs.declarativeHealthcheckPath, container, cs.gardenClientFactory, diff --git a/depot/containerstore/containerstore_test.go b/depot/containerstore/containerstore_test.go index fde81994..2bf3f0fa 100644 --- a/depot/containerstore/containerstore_test.go +++ b/depot/containerstore/containerstore_test.go @@ -160,7 +160,6 @@ var _ = Describe("Container Store", func() { "/var/vcap/data/cf-system-trusted-certs", metronClient, rootFSSizer, - false, "/var/vcap/packages/healthcheck", proxyManager, cellID, @@ -491,7 +490,6 @@ var _ = Describe("Container Store", func() { "/var/vcap/data/cf-system-trusted-certs", metronClient, rootFSSizer, - false, "/var/vcap/packages/healthcheck", proxyManager, cellID, @@ -712,7 +710,7 @@ var _ = Describe("Container Store", func() { Expect(gardenClient.CreateCallCount()).To(Equal(1)) containerSpec := gardenClient.CreateArgsForCall(0) - Expect(containerSpec.BindMounts).NotTo(ContainElement(garden.BindMount{ + Expect(containerSpec.BindMounts).To(ContainElement(garden.BindMount{ SrcPath: "/var/vcap/packages/healthcheck", DstPath: "/etc/cf-assets/healthcheck", Mode: garden.BindMountModeRO, @@ -736,7 +734,6 @@ var _ = Describe("Container Store", func() { "/var/vcap/data/cf-system-trusted-certs", metronClient, rootFSSizer, - true, "/var/vcap/packages/healthcheck", proxyManager, cellID, @@ -1278,7 +1275,6 @@ var _ = Describe("Container Store", func() { "/var/vcap/data/cf-system-trusted-certs", metronClient, rootFSSizer, - false, "/var/vcap/packages/healthcheck", proxyManager, cellID, @@ -1369,7 +1365,6 @@ var _ = Describe("Container Store", func() { "/var/vcap/data/cf-system-trusted-certs", metronClient, rootFSSizer, - false, "/var/vcap/packages/healthcheck", proxyManager, cellID, @@ -2519,7 +2514,6 @@ var _ = Describe("Container Store", func() { "/var/vcap/data/cf-system-trusted-certs", metronClient, rootFSSizer, - false, "/var/vcap/packages/healthcheck", proxyManager, cellID, @@ -3093,7 +3087,6 @@ var _ = Describe("Container Store", func() { "/var/vcap/data/cf-system-trusted-certs", metronClient, rootFSSizer, - false, "/var/vcap/packages/healthcheck", proxyManager, cellID, diff --git a/depot/containerstore/storenode.go b/depot/containerstore/storenode.go index 4607999d..723a9d38 100644 --- a/depot/containerstore/storenode.go +++ b/depot/containerstore/storenode.go @@ -80,7 +80,6 @@ type storeNode struct { process ifrit.Process config *ContainerConfig rootFSSizer configuration.RootFSSizer - useDeclarativeHealthCheck bool declarativeHealthcheckPath string proxyConfigHandler ProxyManager bindMounts []garden.BindMount @@ -100,7 +99,6 @@ type storeNode struct { func newStoreNode( config *ContainerConfig, - useDeclarativeHealthCheck bool, declarativeHealthcheckPath string, container executor.Container, gardenClientFactory GardenClientFactory, @@ -139,7 +137,6 @@ func newStoreNode( modifiedIndex: 0, hostTrustedCertificatesPath: hostTrustedCertificatesPath, metronClient: metronClient, - useDeclarativeHealthCheck: useDeclarativeHealthCheck, declarativeHealthcheckPath: declarativeHealthcheckPath, proxyConfigHandler: proxyConfigHandler, rootFSSizer: rootFSSizer, @@ -262,14 +259,12 @@ func (n *storeNode) Create(logger lager.Logger, traceID string) error { info.Env = append(info.Env, envs...) - if n.useDeclarativeHealthCheck { - logger.Info("adding-healthcheck-bindmounts") - n.bindMounts = append(n.bindMounts, garden.BindMount{ - Origin: garden.BindMountOriginHost, - SrcPath: n.declarativeHealthcheckPath, - DstPath: "/etc/cf-assets/healthcheck", - }) - } + logger.Info("adding-healthcheck-bindmounts") + n.bindMounts = append(n.bindMounts, garden.BindMount{ + Origin: garden.BindMountOriginHost, + SrcPath: n.declarativeHealthcheckPath, + DstPath: "/etc/cf-assets/healthcheck", + }) sourceName, tags := n.info.LogConfig.GetSourceNameAndTagsForLogging() metricErr := n.metronClient.SendAppLog(fmt.Sprintf("Cell %s creating container for instance %s", n.cellID, n.Info().Guid), sourceName, tags) diff --git a/depot/transformer/transformer.go b/depot/transformer/transformer.go index 366f4fe1..601e5ffa 100644 --- a/depot/transformer/transformer.go +++ b/depot/transformer/transformer.go @@ -2,7 +2,6 @@ package transformer import ( "bytes" - "code.cloudfoundry.org/lager/v3" "errors" "fmt" "os" @@ -10,6 +9,8 @@ import ( "strconv" "time" + "code.cloudfoundry.org/lager/v3" + "code.cloudfoundry.org/archiver/compressor" "code.cloudfoundry.org/bbs/models" "code.cloudfoundry.org/cacheddownloader" @@ -56,7 +57,6 @@ type transformer struct { clock clock.Clock sidecarRootFS string - useDeclarativeHealthCheck bool declarativeHealthCheckDefaultTimeout time.Duration emitHealthCheckMetrics bool healthyMonitoringInterval time.Duration @@ -85,7 +85,6 @@ func WithSidecarRootfs( func WithDeclarativeHealthChecks(timeout time.Duration) Option { return func(t *transformer) { - t.useDeclarativeHealthCheck = true if timeout <= 0 { timeout = time.Duration(DefaultDeclarativeHealthcheckRequestTimeout) * time.Millisecond @@ -459,7 +458,7 @@ func (t *transformer) StepsRunner( var proxyStartupChecks []ifrit.Runner var proxyLivenessChecks []ifrit.Runner - if t.useContainerProxy && t.useDeclarativeHealthCheck { + if t.useContainerProxy { envoyStartupLogger := logger.Session("envoy-startup-check") envoyLivenessLogger := logger.Session("envoy-liveness-check") @@ -511,7 +510,7 @@ func (t *transformer) StepsRunner( } } var readinessChan chan steps.ReadinessState - if container.CheckDefinition != nil && t.useDeclarativeHealthCheck { + if container.CheckDefinition != nil { if container.CheckDefinition.Checks != nil { monitor = t.transformCheckDefinition(logger, &container, diff --git a/depot/transformer/transformer_test.go b/depot/transformer/transformer_test.go index fa6abd61..be9d77d8 100644 --- a/depot/transformer/transformer_test.go +++ b/depot/transformer/transformer_test.go @@ -1,7 +1,6 @@ package transformer_test import ( - "code.cloudfoundry.org/durationjson" "errors" "fmt" "io" @@ -13,6 +12,8 @@ import ( "sync/atomic" "time" + "code.cloudfoundry.org/durationjson" + "code.cloudfoundry.org/bbs/models" "code.cloudfoundry.org/clock/fakeclock" mfakes "code.cloudfoundry.org/diego-logging-client/testhelpers" @@ -291,12 +292,18 @@ var _ = Describe("Transformer", func() { Describe("container proxy", func() { var ( - container executor.Container - processLock sync.Mutex - process ifrit.Process - envoyProcess *gardenfakes.FakeProcess - actionCh chan int - envoyCh chan int + container executor.Container + processLock sync.Mutex + process ifrit.Process + envoyProcess *gardenfakes.FakeProcess + actionCh chan int + envoyCh chan int + startupIO chan garden.ProcessIO + livenessIO chan garden.ProcessIO + startupCh chan int + startupProcess *gardenfakes.FakeProcess + livenessProcess *gardenfakes.FakeProcess + livenessCh chan int ) BeforeEach(func() { @@ -311,6 +318,18 @@ var _ = Describe("Transformer", func() { envoyCh = make(chan int) envoyProcess = makeProcess(envoyCh) + startupIO = make(chan garden.ProcessIO, 1) + livenessIO = make(chan garden.ProcessIO, 1) + startupIOCh := startupIO + livenessIOCh := livenessIO + + startupCh = make(chan int) + startupProcess = makeProcess(startupCh) + + livenessCh = make(chan int, 1) + livenessProcess = makeProcess(livenessCh) + + healthcheckCallCount := int64(0) gardenContainer.RunStub = func(spec garden.ProcessSpec, io garden.ProcessIO) (process garden.Process, err error) { defer GinkgoRecover() // get rid of race condition caused by write inside the BeforeEach @@ -325,6 +344,16 @@ var _ = Describe("Transformer", func() { return envoyProcess, nil case spec.Path == "/etc/cf-assets/envoy/envoy" && runtime.GOOS == "windows": return envoyProcess, nil + case spec.Path == filepath.Join(transformer.HealthCheckDstPath, "healthcheck"): + oldCount := atomic.AddInt64(&healthcheckCallCount, 1) + switch oldCount { + case 1: + startupIOCh <- io + return startupProcess, nil + case 2: + livenessIOCh <- io + return livenessProcess, nil + } } err = errors.New("") @@ -382,11 +411,13 @@ var _ = Describe("Transformer", func() { AfterEach(func() { close(actionCh) close(envoyCh) + close(startupCh) + livenessCh <- 1 ginkgomon.Interrupt(process) }) It("runs the container proxy in a sidecar container", func() { - Eventually(gardenContainer.RunCallCount).Should(Equal(2)) + Eventually(gardenContainer.RunCallCount).Should(Equal(3)) specs := []garden.ProcessSpec{} for i := 0; i < gardenContainer.RunCallCount(); i++ { spec, _ := gardenContainer.RunArgsForCall(i) @@ -435,7 +466,7 @@ var _ = Describe("Transformer", func() { Context("when the process is signalled", func() { JustBeforeEach(func() { - Eventually(gardenContainer.RunCallCount).Should(Equal(2)) + Eventually(gardenContainer.RunCallCount).Should(Equal(3)) process.Signal(os.Interrupt) }) @@ -446,7 +477,7 @@ var _ = Describe("Transformer", func() { Context("when the envoy process is signalled", func() { JustBeforeEach(func() { - Eventually(gardenContainer.RunCallCount).Should(Equal(2)) + Eventually(gardenContainer.RunCallCount).Should(Equal(3)) envoyCh <- 134 }) @@ -459,6 +490,7 @@ var _ = Describe("Transformer", func() { It("process should fail with a descriptive error", func() { actionCh <- 0 + startupCh <- 0 Eventually(process.Wait()).Should(Receive(MatchError("PROXY: Exited with status 134"))) }) }) @@ -469,7 +501,7 @@ var _ = Describe("Transformer", func() { }) It("runs the container proxy in a sidecar container", func() { - Eventually(gardenContainer.RunCallCount).Should(Equal(2)) + Eventually(gardenContainer.RunCallCount).Should(Equal(3)) specs := []garden.ProcessSpec{} for i := 0; i < gardenContainer.RunCallCount(); i++ { spec, _ := gardenContainer.RunArgsForCall(i) @@ -523,7 +555,7 @@ var _ = Describe("Transformer", func() { }) It("does not run the container proxy", func() { - Eventually(gardenContainer.RunCallCount).Should(Equal(1)) + Eventually(gardenContainer.RunCallCount).Should(Equal(2)) paths := []string{} for i := 0; i < gardenContainer.RunCallCount(); i++ { spec, _ := gardenContainer.RunArgsForCall(i) diff --git a/initializer/initializer.go b/initializer/initializer.go index 30e90e1e..85b6c87a 100644 --- a/initializer/initializer.go +++ b/initializer/initializer.go @@ -103,7 +103,6 @@ type ExecutorConfig struct { DiskMB string `json:"disk_mb,omitempty"` EnableContainerProxy bool `json:"enable_container_proxy,omitempty"` EnableContainerProxyHealthChecks bool `json:"enable_container_proxy_healthcheck,omitempty"` - EnableDeclarativeHealthcheck bool `json:"enable_declarative_healthcheck,omitempty"` DeclarativeHealthCheckDefaultTimeout durationjson.Duration `json:"declarative_healthcheck_default_timeout,omitempty"` EnableHealtcheckMetrics bool `json:"enable_healthcheck_metrics,omitempty"` EnableUnproxiedPortMappings bool `json:"enable_unproxied_port_mappings"` @@ -262,7 +261,6 @@ func Initialize( clock, postSetupHook, config.PostSetupUser, - config.EnableDeclarativeHealthcheck, config.EnableHealtcheckMetrics, sidecarRootFSPath, config.EnableContainerProxy, @@ -348,7 +346,6 @@ func Initialize( config.TrustedSystemCertificatesPath, metronClient, rootFSSizer, - config.EnableDeclarativeHealthcheck, config.DeclarativeHealthcheckPath, proxyConfigHandler, cellID, @@ -569,7 +566,6 @@ func initializeTransformer( clock clock.Clock, postSetupHook []string, postSetupUser string, - useDeclarativeHealthCheck bool, emitHealthCheckMetrics bool, declarativeHealthcheckRootFS string, enableContainerProxy bool, @@ -584,9 +580,7 @@ func initializeTransformer( options = append(options, transformer.WithSidecarRootfs(declarativeHealthcheckRootFS)) - if useDeclarativeHealthCheck { - options = append(options, transformer.WithDeclarativeHealthChecks(declarativeHealthCheckDefaultTimeout)) - } + options = append(options, transformer.WithDeclarativeHealthChecks(declarativeHealthCheckDefaultTimeout)) if emitHealthCheckMetrics { options = append(options, transformer.WithDeclarativeHealthcheckFailureMetrics()) @@ -771,7 +765,7 @@ func (config *ExecutorConfig) Validate(logger lager.Logger) bool { valid = false } - if config.EnableDeclarativeHealthcheck && time.Duration(config.DeclarativeHealthCheckDefaultTimeout) <= 0 { + if time.Duration(config.DeclarativeHealthCheckDefaultTimeout) <= 0 { logger.Error("declarative_healthcheck_default_timeout", nil) valid = false } diff --git a/initializer/initializer_test.go b/initializer/initializer_test.go index 74c27218..6d7a07e6 100644 --- a/initializer/initializer_test.go +++ b/initializer/initializer_test.go @@ -80,7 +80,6 @@ var _ = Describe("Initializer", func() { DeleteWorkPoolSize: 32, DiskMB: configuration.Automatic, EnableContainerProxy: false, - EnableDeclarativeHealthcheck: false, GardenAddr: "/tmp/garden.sock", GardenHealthcheckCommandRetryPause: durationjson.Duration(1 * time.Second), GardenHealthcheckEmissionInterval: durationjson.Duration(30 * time.Second),