Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions depot/containerstore/containerstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ type containerStore struct {
rootFSSizer configuration.RootFSSizer
logManager LogManager

useDeclarativeHealthCheck bool
declarativeHealthcheckPath string

proxyConfigHandler ProxyManager
Expand Down Expand Up @@ -106,7 +105,6 @@ func New(
trustedSystemCertificatesPath string,
metronClient loggingclient.IngressClient,
rootFSSizer configuration.RootFSSizer,
useDeclarativeHealthCheck bool,
declarativeHealthcheckPath string,
proxyConfigHandler ProxyManager,
cellID string,
Expand All @@ -129,7 +127,6 @@ func New(
metronClient: metronClient,
rootFSSizer: rootFSSizer,
trustedSystemCertificatesPath: trustedSystemCertificatesPath,
useDeclarativeHealthCheck: useDeclarativeHealthCheck,
declarativeHealthcheckPath: declarativeHealthcheckPath,
proxyConfigHandler: proxyConfigHandler,

Expand All @@ -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,
Expand Down
9 changes: 1 addition & 8 deletions depot/containerstore/containerstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 6 additions & 11 deletions depot/containerstore/storenode.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ type storeNode struct {
process ifrit.Process
config *ContainerConfig
rootFSSizer configuration.RootFSSizer
useDeclarativeHealthCheck bool
declarativeHealthcheckPath string
proxyConfigHandler ProxyManager
bindMounts []garden.BindMount
Expand All @@ -100,7 +99,6 @@ type storeNode struct {

func newStoreNode(
config *ContainerConfig,
useDeclarativeHealthCheck bool,
declarativeHealthcheckPath string,
container executor.Container,
gardenClientFactory GardenClientFactory,
Expand Down Expand Up @@ -139,7 +137,6 @@ func newStoreNode(
modifiedIndex: 0,
hostTrustedCertificatesPath: hostTrustedCertificatesPath,
metronClient: metronClient,
useDeclarativeHealthCheck: useDeclarativeHealthCheck,
declarativeHealthcheckPath: declarativeHealthcheckPath,
proxyConfigHandler: proxyConfigHandler,
rootFSSizer: rootFSSizer,
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 4 additions & 5 deletions depot/transformer/transformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@ package transformer

import (
"bytes"
"code.cloudfoundry.org/lager/v3"
"errors"
"fmt"
"os"
"path/filepath"
"strconv"
"time"

"code.cloudfoundry.org/lager/v3"

"code.cloudfoundry.org/archiver/compressor"
"code.cloudfoundry.org/bbs/models"
"code.cloudfoundry.org/cacheddownloader"
Expand Down Expand Up @@ -56,7 +57,6 @@ type transformer struct {
clock clock.Clock

sidecarRootFS string
useDeclarativeHealthCheck bool
declarativeHealthCheckDefaultTimeout time.Duration
emitHealthCheckMetrics bool
healthyMonitoringInterval time.Duration
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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,
Expand Down
56 changes: 44 additions & 12 deletions depot/transformer/transformer_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package transformer_test

import (
"code.cloudfoundry.org/durationjson"
"errors"
"fmt"
"io"
Expand All @@ -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"
Expand Down Expand Up @@ -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() {
Expand All @@ -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
Expand All @@ -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("")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
})

Expand All @@ -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
})

Expand All @@ -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")))
})
})
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 2 additions & 8 deletions initializer/initializer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -262,7 +261,6 @@ func Initialize(
clock,
postSetupHook,
config.PostSetupUser,
config.EnableDeclarativeHealthcheck,
config.EnableHealtcheckMetrics,
sidecarRootFSPath,
config.EnableContainerProxy,
Expand Down Expand Up @@ -348,7 +346,6 @@ func Initialize(
config.TrustedSystemCertificatesPath,
metronClient,
rootFSSizer,
config.EnableDeclarativeHealthcheck,
config.DeclarativeHealthcheckPath,
proxyConfigHandler,
cellID,
Expand Down Expand Up @@ -569,7 +566,6 @@ func initializeTransformer(
clock clock.Clock,
postSetupHook []string,
postSetupUser string,
useDeclarativeHealthCheck bool,
emitHealthCheckMetrics bool,
declarativeHealthcheckRootFS string,
enableContainerProxy bool,
Expand All @@ -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())
Expand Down Expand Up @@ -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
}
Expand Down
1 change: 0 additions & 1 deletion initializer/initializer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down