Refactor VolumeMountPath to Directory

[davidhadas]

Proposed Changes

• Name refactoring
• Add exported const for Annotations filepath

[davidhadas]

@rhuss rhuss 9 hours ago
minor nit pick: From a user's point of view, it does not matter whether a directory is mounted or part or a original directory of that file directory. Therefore I really would prefer "directory" instead of "mountPath" except when you would actually would do the mount yourself (I doubt that). I would better rename the other MountPath constants to use Directory instead (like PodInfoVolumeMountPath)

@davidhadas davidhadas 4 hours ago •
Fine either way, as long as all mount points use the same convention.

Note however that we added a config feature to control if podInfo is mounted or not - this is also reflected in the documentation as a result - so we do expose this externally as a mounted volume.

If we open a new PR for this, I would like to also add a PodInfoAnnotationsPath (or we can call it AnnotationsPath) const in shardmain - this allows Options importing shardmain to use the shardmain const rather than importing also pkg/queue to gain access to the consts defining the annotations file path to open and read.

@psschwei psschwei 3 hours ago
"directory" works for me (it also seems more in line with the spirit of these variables, as the original comments note these are for "the name of the directory path")

[davidhadas]

/cc @rhuss @psschwei

[codecov]

Codecov Report

Merging #13202 (15d8a2e) into main (5bba016) will decrease coverage by 0.10%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main   #13202      +/-   ##
==========================================
- Coverage   86.72%   86.62%   -0.11%     
==========================================
  Files         196      196              
  Lines       14463    14493      +30     
==========================================
+ Hits        12543    12554      +11     
- Misses       1624     1640      +16     
- Partials      296      299       +3     

Impacted Files	Coverage Δ
pkg/queue/sharedmain/main.go	0.64% <ø> (-0.01%)	⬇️
pkg/reconciler/revision/resources/deploy.go	90.54% <ø> (ø)
pkg/reconciler/revision/resources/queue.go	98.23% <100.00%> (+0.09%)	⬆️
pkg/http/handler/timeout.go	83.44% <0.00%> (-6.36%)	⬇️
pkg/reconciler/configuration/configuration.go	83.67% <0.00%> (-1.54%)	⬇️
pkg/apis/config/defaults.go	84.48% <0.00%> (-1.24%)	⬇️
pkg/activator/net/revision_backends.go	92.19% <0.00%> (-0.58%)	⬇️
pkg/apis/serving/v1/revision_types.go	100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[dprotaso]

/lgtm
/approve

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidhadas, dprotaso

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• pkg/OWNERS [dprotaso]

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

[psschwei]

/test upgrade-tests_serving_main

[dprotaso]

/retest

[davidhadas]

/retest