pkg/timezone: add helper function to configure timezone for pod/containers #1775
Conversation
sohankunkerkar
force-pushed
the
add-timezone-helper-fns
branch
from
7383db4 to
ce7fce5
Compare
|
I agree lets not further polute util, create a new package. |
|
Does it make sense to include this under |
|
Perhaps if you rename the pkg to pkg/time |
|
I would suggest pkg/timezone personally to make it abundantly clear what's happening |
sohankunkerkar
force-pushed
the
add-timezone-helper-fns
branch
from
ce7fce5 to
b87c410
Compare
sohankunkerkar
changed the title
sohankunkerkar
force-pushed
the
add-timezone-helper-fns
branch
from
b87c410 to
5e15c0c
Compare
sohankunkerkar
changed the title
pkg/timezone/timezone_linux.go
Outdated
|
|
||
| // ConfigureContainerTimeZone configure the time zone for a container. | ||
| // It returns the path of the created /etc/localtime file if needed. | ||
| func ConfigureContainerTimeZone(timezone, containerRunDir, mountPoint, etcPath, mountLabel, containerID string) (string, error) { |
There was a problem hiding this comment.
Can we add tests for it or make it testable?
There was a problem hiding this comment.
Due to the nature of the ConfigureContainerTimeZone function, it's challenging to write unit tests without proper isolation or mocking of the system calls involved. Specifically, the unix.Unlinkat and unix.Symlinkat functions interact directly with the file system, which makes it difficult to predict their behavior in a controlled test environment.
sohankunkerkar
force-pushed
the
add-timezone-helper-fns
branch
from
5e15c0c to
a79217b
Compare
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan, sohankunkerkar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…ainers This PR consolidates common functionality used by CRI-O and Podman in one central location. I aimed to keep this change more generic, considering that CRI-O and Podman have different ways for the file mounting and applying security labels. Signed-off-by: Sohan Kunkerkar <[email protected]>
sohankunkerkar
force-pushed
the
add-timezone-helper-fns
branch
from
a79217b to
1a7c739
Compare
|
LGTM, @saschagrunert WDYT? |
|
/lgtm |
|
@rhatdan is it possible to get a minor version release for this change? |
|
podman 5.0 will be released in February. Rolling this through to a podman minor release is not likely. |
|
@rhatdan Thanks for the information. Actually, we need this release for CRI-O. I attempted to vendor the main changes from |
[0.57] backport: #1775
This PR consolidates common functionality used by CRI-O and Podman in one central location. I aimed to keep this change more generic, considering that CRI-O and Podman have different ways for the file mounting and applying security labels.
Related to containers/podman#21063 (comment)