[WIP] Drop /var/log support

cmd/queue/main.go

@@ -27,9 +27,7 @@ import (
  "net/http/httputil"
  "net/url"
  "os"
- "path"
  "strconv"
- "strings"
  "time"
 
  "github.com/kelseyhightower/envconfig"
@@ -111,12 +109,6 @@ type config struct {
  ServingService string `split_words:"true"` // optional
  ServingRequestMetricsBackend string `split_words:"true"` // optional
 
- // /var/log configuration
- EnableVarLogCollection bool `split_words:"true"` // optional
- UserContainerName string `split_words:"true"` // optional
- VarLogVolumeName string `split_words:"true"` // optional
- InternalVolumePath string `split_words:"true"` // optional
-
  // DownwardAPI configuration for pod labels
  DownwardAPILabelsPath string `split_words:"true"`
 
@@ -348,10 +340,6 @@ func main() {
  }.String()),
  zap.String(logkey.Pod, env.ServingPod))
 
- if err := validateEnv(env); err != nil {
- logger.Fatal(err.Error())
- }
-
  // Report stats on Go memory usage every 30 seconds.
  msp := metrics.NewMemStatsAll()
  msp.Start(context.Background(), 30*time.Second)
@@ -403,15 +391,6 @@ func main() {
  }(name, server)
  }
 
- // Setup /var/log.
- // Logic that isn't required to be executed before the critical path
- // and should be started last to not impact start up latency
- go func() {
- if env.EnableVarLogCollection {
- createVarLogLink(env)
- }
- }()
-
  // Blocks until we actually receive a TERM signal or one of the servers
  // exit unexpectedly. We fold both signals together because we only want
  // to act on the first of those to reach here.
@@ -447,21 +426,6 @@ func main() {
  }
 }
 
-func validateEnv(env config) error {
- if !env.EnableVarLogCollection {
- return nil
- }
-
- if env.VarLogVolumeName == "" {
- return errors.New("VAR_LOG_VOLUME_NAME must be specified when ENABLE_VAR_LOG_COLLECTION is true")
- }
- if env.InternalVolumePath == "" {
- return errors.New("INTERNAL_VOLUME_PATH must be specified when ENABLE_VAR_LOG_COLLECTION is true")
- }
-
- return nil
-}
-
 func buildProbe(probeJSON string) *readiness.Probe {
  coreProbe, err := readiness.DecodeProbe(probeJSON)
  if err != nil {
@@ -581,17 +545,6 @@ func buildMetricsServer(promStatReporter *queue.PrometheusStatsReporter) *http.S
  }
 }
 
-// createVarLogLink creates a symlink allowing the fluentd daemon set to capture the
-// logs from the user container /var/log. See fluentd config for more details.
-func createVarLogLink(env config) {
- link := strings.Join([]string{env.ServingNamespace, env.ServingPod, env.UserContainerName}, "_")
- target := path.Join("..", env.VarLogVolumeName)
- source := path.Join(env.InternalVolumePath, link)
- if err := os.Symlink(target, source); err != nil {
- logger.Errorw("Failed to create /var/log symlink. Log collection will not work.", zap.Error(err))
- }
-}
-
 func pushRequestLogHandler(currentHandler http.Handler, env config) http.Handler {
  if env.ServingRequestLogTemplate == "" {
  return currentHandler

cmd/queue/main_test.go

@@ -23,7 +23,6 @@ import (
  "net/http/httputil"
  "net/url"
  "os"
- "path"
  "strconv"
  "strings"
  "sync/atomic"
@@ -172,32 +171,6 @@ func TestProbeHandler(t *testing.T) {
  }
 }
 
-func TestCreateVarLogLink(t *testing.T) {
- dir, err := ioutil.TempDir("", "TestCreateVarLogLink")
- if err != nil {
- t.Errorf("Failed to created temporary directory: %v", err)
- }
- defer os.RemoveAll(dir)
- var env = config{
- ServingNamespace: "default",
- ServingPod: "service-7f97f9465b-5kkm5",
- UserContainerName: "user-container",
- VarLogVolumeName: "knative-var-log",
- InternalVolumePath: dir,
- }
- createVarLogLink(env)
-
- source := path.Join(dir, "default_service-7f97f9465b-5kkm5_user-container")
- want := "../knative-var-log"
- got, err := os.Readlink(source)
- if err != nil {
- t.Errorf("Failed to read symlink: %v", err)
- }
- if got != want {
- t.Errorf("Incorrect symlink = %q, want %q, diff: %s", got, want, cmp.Diff(got, want))
- }
-}
-
 func TestProbeQueueInvalidPort(t *testing.T) {
  const port = 0 // invalid port
 

docs/runtime-contract.md

@@ -426,10 +426,10 @@ In addition to the filesystems recommended in the OCI, the following filesystems
 [MUST](https://github.com/knative/serving/blob/master/test/conformance/runtime/filesystem_perm_test.go)
 be provided:
 
-| Mount | Description  |
-| ---------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `/tmp` | MUST be Read-write.<p>SHOULD be backed by tmpfs if disk load is a concern.  |
-| `/var/log` | MUST be a directory with write permissions for logs storage. Implementations MAY permit the creation of additional subdirectories and log rotation and renaming. |
+| Mount | Description |
+| ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `/tmp` | MUST be Read-write.<p>SHOULD be backed by tmpfs if disk load is a concern. |
+| `/var/log` | MAY be a directory with write permissions for logs storage. Implementations MAY permit the creation of additional subdirectories and log rotation and renaming. |
 
 To enable DNS resolution, the following files might be overwritten at runtime:
 

pkg/metrics/config_test.go

@@ -51,19 +51,17 @@ func TestObservabilityConfiguration(t *testing.T) {
  name: "observability configuration with all inputs",
  wantErr: false,
  wantController: &metrics.ObservabilityConfig{
- LoggingURLTemplate: "https://logging.io",
- EnableVarLogCollection: true,
- RequestLogTemplate: `{"requestMethod": "{{.Request.Method}}"}`,
- EnableProbeRequestLog: true,
- RequestMetricsBackend: "stackdriver",
+ LoggingURLTemplate: "https://logging.io",
+ RequestLogTemplate: `{"requestMethod": "{{.Request.Method}}"}`,
+ EnableProbeRequestLog: true,
+ RequestMetricsBackend: "stackdriver",
  },
  config: &corev1.ConfigMap{
  ObjectMeta: metav1.ObjectMeta{
  Namespace: system.Namespace(),
  Name: metrics.ConfigMapName(),
  },
  Data: map[string]string{
- "logging.enable-var-log-collection": "true",
  "logging.revision-url-template": "https://logging.io",
  "logging.enable-probe-request-log": "true",
  "logging.write-request-logs": "true",
@@ -75,10 +73,9 @@ func TestObservabilityConfiguration(t *testing.T) {
  name: "observability config with no map",
  wantErr: false,
  wantController: &metrics.ObservabilityConfig{
- EnableVarLogCollection: false,
- LoggingURLTemplate: metrics.DefaultLogURLTemplate,
- RequestLogTemplate: "",
- RequestMetricsBackend: "prometheus",
+ LoggingURLTemplate: metrics.DefaultLogURLTemplate,
+ RequestLogTemplate: "",
+ RequestMetricsBackend: "prometheus",
  },
  config: &corev1.ConfigMap{
  ObjectMeta: metav1.ObjectMeta{

pkg/reconciler/revision/resources/deploy.go

@@ -42,29 +42,13 @@ import (
 )
 
 const (
- varLogVolumeName = "knative-var-log"
- varLogVolumePath = "/var/log"
- internalVolumeName = "knative-internal"
- internalVolumePath = "/var/knative-internal"
  podInfoVolumeName = "podinfo"
  podInfoVolumePath = "/etc/podinfo"
  metadataLabelsRef = "metadata.labels"
  metadataLabelsPath = "labels"
 )
 
 var (
- varLogVolume = corev1.Volume{
- Name: varLogVolumeName,
- VolumeSource: corev1.VolumeSource{
- EmptyDir: &corev1.EmptyDirVolumeSource{},
- },
- }
-
- varLogVolumeMount = corev1.VolumeMount{
- Name: varLogVolumeName,
- MountPath: varLogVolumePath,
- }
-
  labelVolume = corev1.Volume{
  Name: podInfoVolumeName,
  VolumeSource: corev1.VolumeSource{
@@ -86,18 +70,6 @@ var (
  MountPath: podInfoVolumePath,
  }
 
- internalVolume = corev1.Volume{
- Name: internalVolumeName,
- VolumeSource: corev1.VolumeSource{
- EmptyDir: &corev1.EmptyDirVolumeSource{},
- },
- }
-
- internalVolumeMount = corev1.VolumeMount{
- Name: internalVolumeName,
- MountPath: internalVolumePath,
- }
-
  // This PreStop hook is actually calling an endpoint on the queue-proxy
  // because of the way PreStop hooks are called by kubelet. We use this
  // to block the user-container from exiting before the queue-proxy is ready
@@ -144,11 +116,6 @@ func makePodSpec(rev *v1.Revision, loggingConfig *logging.Config, tracingConfig
  userContainer := BuildUserContainer(rev)
  podSpec := BuildPodSpec(rev, []corev1.Container{*userContainer, *queueContainer})
 
- // Add the Knative internal volume only if /var/log collection is enabled
- if observabilityConfig.EnableVarLogCollection {
- podSpec.Volumes = append(podSpec.Volumes, internalVolume)
- }
-
  if autoscalerConfig.EnableGracefulScaledown {
  podSpec.Volumes = append(podSpec.Volumes, labelVolume)
  }
@@ -162,7 +129,6 @@ func BuildUserContainer(rev *v1.Revision) *corev1.Container {
  // Adding or removing an overwritten corev1.Container field here? Don't forget to
  // update the fieldmasks / validations in pkg/apis/serving
 
- userContainer.VolumeMounts = append(userContainer.VolumeMounts, varLogVolumeMount)
  userContainer.Lifecycle = userLifecycle
  userPort := getUserPort(rev)
  userPortInt := int(userPort)
@@ -201,7 +167,7 @@ func BuildUserContainer(rev *v1.Revision) *corev1.Container {
 func BuildPodSpec(rev *v1.Revision, containers []corev1.Container) *corev1.PodSpec {
  return &corev1.PodSpec{
  Containers: containers,
- Volumes: append([]corev1.Volume{varLogVolume}, rev.Spec.Volumes...),
+ Volumes: rev.Spec.Volumes,
  ServiceAccountName: rev.Spec.ServiceAccountName,
  TerminationGracePeriodSeconds: rev.Spec.TimeoutSeconds,
  ImagePullSecrets: rev.Spec.ImagePullSecrets,

pkg/reconciler/revision/resources/deploy_test.go

@@ -49,7 +49,6 @@ var (
  Name: containerName,
  Image: "busybox",
  Ports: buildContainerPorts(v1.DefaultUserPort),
- VolumeMounts: []corev1.VolumeMount{varLogVolumeMount},
  Lifecycle: userLifecycle,
  TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
  Stdin: false,
@@ -147,18 +146,6 @@ var (
  }, {
  Name: "METRICS_DOMAIN",
  Value: metrics.Domain(),
- }, {
- Name: "USER_CONTAINER_NAME",
- Value: containerName,
- }, {
- Name: "ENABLE_VAR_LOG_COLLECTION",
- Value: "false",
- }, {
- Name: "VAR_LOG_VOLUME_NAME",
- Value: varLogVolumeName,
- }, {
- Name: "INTERNAL_VOLUME_PATH",
- Value: internalVolumePath,
  }, {
  Name: "DOWNWARD_API_LABELS_PATH",
  Value: fmt.Sprintf("%s/%s", podInfoVolumePath, metadataLabelsPath),
@@ -175,7 +162,6 @@ var (
  }
 
  defaultPodSpec = &corev1.PodSpec{
- Volumes: []corev1.Volume{varLogVolume},
  TerminationGracePeriodSeconds: refInt64(45),
  }
 
@@ -276,12 +262,6 @@ func withEnvVar(name, value string) containerOption {
  }
 }
 
-func withInternalVolumeMount() containerOption {
- return func(container *corev1.Container) {
- container.VolumeMounts = append(container.VolumeMounts, internalVolumeMount)
- }
-}
-
 func withPodLabelsVolumeMount() containerOption {
  return func(container *corev1.Container) {
  container.VolumeMounts = append(container.VolumeMounts, labelVolumeMount)
@@ -635,30 +615,6 @@ func TestMakePodSpec(t *testing.T) {
  withEnvVar("CONTAINER_CONCURRENCY", "0"),
  ),
  }),
- }, {
- name: "with /var/log collection",
- rev: revision("bar", "foo", withContainerConcurrency(1),
- func(revision *v1.Revision) {
- container(revision.Spec.GetContainer(),
- withTCPReadinessProbe(),
- )
- }),
- oc: metrics.ObservabilityConfig{
- EnableVarLogCollection: true,
- },
- want: podSpec(
- []corev1.Container{
- userContainer(),
- queueContainer(
- withEnvVar("CONTAINER_CONCURRENCY", "1"),
- withEnvVar("ENABLE_VAR_LOG_COLLECTION", "true"),
- withInternalVolumeMount(),
- ),
- },
- func(podSpec *corev1.PodSpec) {
- podSpec.Volumes = append(podSpec.Volumes, internalVolume)
- },
- ),
  }, {
  name: "with graceful scaledown enabled",
  rev: revision("bar", "foo", func(revision *v1.Revision) {

pkg/reconciler/revision/resources/queue.go

@@ -220,9 +220,6 @@ func makeQueueContainer(rev *v1.Revision, loggingConfig *logging.Config, tracing
  ports = append(ports, servingPort)
 
  var volumeMounts []corev1.VolumeMount
- if observabilityConfig.EnableVarLogCollection {
- volumeMounts = append(volumeMounts, internalVolumeMount)
- }
 
  if autoscalerConfig.EnableGracefulScaledown {
  volumeMounts = append(volumeMounts, labelVolumeMount)
@@ -317,18 +314,6 @@ func makeQueueContainer(rev *v1.Revision, loggingConfig *logging.Config, tracing
  }, {
  Name: pkgmetrics.DomainEnv,
  Value: pkgmetrics.Domain(),
- }, {
- Name: "USER_CONTAINER_NAME",
- Value: rev.Spec.GetContainer().Name,
- }, {
- Name: "ENABLE_VAR_LOG_COLLECTION",
- Value: strconv.FormatBool(observabilityConfig.EnableVarLogCollection),
- }, {
- Name: "VAR_LOG_VOLUME_NAME",
- Value: varLogVolumeName,
- }, {
- Name: "INTERNAL_VOLUME_PATH",
- Value: internalVolumePath,
  }, {
  Name: "DOWNWARD_API_LABELS_PATH",
  Value: fmt.Sprintf("%s/%s", podInfoVolumePath, metadataLabelsPath),

pkg/reconciler/revision/resources/queue_test.go

@@ -784,10 +784,6 @@ var defaultEnv = map[string]string{
  "SYSTEM_NAMESPACE": system.Namespace(),
  "METRICS_DOMAIN": metrics.Domain(),
  "QUEUE_SERVING_PORT": "8012",
- "USER_CONTAINER_NAME": containerName,
- "ENABLE_VAR_LOG_COLLECTION": "false",
- "VAR_LOG_VOLUME_NAME": varLogVolumeName,
- "INTERNAL_VOLUME_PATH": internalVolumePath,
  "DOWNWARD_API_LABELS_PATH": fmt.Sprintf("%s/%s", podInfoVolumePath, metadataLabelsPath),
  "ENABLE_PROFILING": "false",
  "SERVING_ENABLE_PROBE_REQUEST_LOG": "false",