[WIP] Drop /var/log support

[dprotaso]

Fixes #6147

Proposed Changes

• Update the runtime contract to make support collecting logs under /var/log` optional
• The /var/log support currently in serving is coupled with the existing monitoring bundle. Since that monitoring bundle is being deprecated were going to drop support for it

Release Note

Collection of logs under /var/log is now optional per the runtime contract and the functionality has been removed

[knative-prow-robot]

@dprotaso: 0 warnings.

In response to this:

Fixes #6147

Proposed Changes

• Update the runtime contract to make support collecting logs under /var/log` optional
• The /var/log support currently in serving is coupled with the existing monitoring bundle. Since that monitoring bundle is being deprecated were going to drop support for it

Release Note

Collection of logs under /var/log is now optional per the runtime contract and the functionality has been removed

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dprotaso
To complete the pull request process, please assign vaikas
You can assign the PR to them by writing /assign @vaikas in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mattmoor]

Produced via:
prettier --write --prose-wrap=always $(find -name '*.md' | grep -v vendor | grep -v .github | grep -v docs/cmd/)

[mattmoor]

Produced via:
prettier --write --prose-wrap=always $(find -name '*.md' | grep -v vendor | grep -v .github | grep -v docs/cmd/)

[dprotaso]

/assign @JRBANCEL

[JRBANCEL]

What is the depreciation process in Knative?
We can't drop a feature like this without knowing who is using it, can we?

[vagababov]

I guess this deserves a mention at the API WG.
But we're relaxing restraints so it's not dropping feature?

[JRBANCEL]

But we're relaxing restraints so it's not dropping feature?

How is removing the code implementing the feature not dropping the feature?

[vagababov]

🤷
I read the optional part :)

[dprotaso]

FYI - I'm in no rush to merge this

This should be paired with the deprecation & removal of the monitoring bundle. This was mentioned in the 0.14.0 release notes

[mattmoor]

Let's discuss tomorrow, but I think we're stuck with this for the duration of the deprecation period.

The monitoring bundle was alpha / PoC, but this was part of our v1 runtime contract, so it's a higher bar.

[dprotaso]

Here's the raw download stats of our release yamls - which might be a rough measure of interest?

See gist
https://gist.github.com/dprotaso/e3caa7a5756c6be66a4f810e131dfa3a

[knative-prow-robot]

@dprotaso: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-knative-serving-build-tests	1bf04be	link	/test pull-knative-serving-build-tests
pull-knative-serving-integration-tests	1bf04be	link	/test pull-knative-serving-integration-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[knative-test-reporter-robot]

The following jobs failed:

Test name	Triggers	Retries
pull-knative-serving-integration-tests	pull-knative-serving-integration-tests pull-knative-serving-integration-tests pull-knative-serving-integration-tests pull-knative-serving-integration-tests	3/3

Job pull-knative-serving-integration-tests expended all 3 retries without success.

[dprotaso]

@JRBANCEL From the WG meeting discussion a suggested approach could be:

1. Make /var/log optional in the runtime contract
2. Instead of dropping support in the upstream we change the behaviour to:
   2.1 When /var/log collection is enabled by the operator we keep the same behaviour as is
   2.2 When /var/log collection is disabled by the operator we do not mount the read/writable /var/log volume
   2.2 We can still drop the queue proxy logic with the subpath expression but it'll require changes to the monitoring bundle (ie. the regex)

For 2.1 we could also move the logic to add the volumes to a webhook

[JRBANCEL]

Sound like a plan 👍

About 2.2, there only problem with this approach is that then enabling the feature has potential to break existing working services.

[dprotaso]

Closing in favour of moving the discussion to the issue #7881