-
Notifications
You must be signed in to change notification settings - Fork 349
Conversation
I went ahead and squashed this as it had many WIP commits and unsigned commits from the previous PR causing CI to fail. |
/test pull-cri-containerd-node-e2e |
e2e bucket timed out for no obvious reason restarting |
I'm not getting good info on why this is timing out.... |
Carry of containerd#1246 Signed-off-by: Darren Shepherd <[email protected]> Signed-off-by: Michael Crosby <[email protected]>
/test pull-cri-containerd-node-e2e it looks like a normal timeout now for a single test, going to rerun |
victory! |
Tested labeling for volumes: system_u:system_r:container_t:s0:c192,c450 root 9226 0.0 0.1 10636 5168 ? Ss 11:57 0:00 nginx: master process nginx -g daemon off;
system_u:system_r:container_t:s0:c192,c450 101 9240 0.0 0.0 11100 2552 ? S 11:57 0:00 nginx: worker process process ^^ drwxr-xr-x. 2 root root unconfined_u:object_r:mnt_t:s0 6 May 26 11:57 mnt
drwxr-xr-x. 2 root root system_u:object_r:container_file_t:s0:c192,c450 6 May 26 11:57 test mount ^^ |
I think we could merge this and then follow up on adding reference counting to the pod level and off of the container. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This moves most of the API calls off of the `labels` package onto the root selinux package. This is the newer API for most selinux operations. Signed-off-by: Michael Crosby <[email protected]>
Updated, please take another look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
c := &criService{ | ||
config: config, | ||
client: client, | ||
os: osinterface.RealOS{}, | ||
sandboxStore: sandboxstore.NewStore(), | ||
containerStore: containerstore.NewStore(), | ||
sandboxStore: sandboxstore.NewStore(labels), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should try to decouple this a little bit, maybe having the label store could be registered through the plugin system in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a follow up or maybe post merge back into containerd
Ya, the store is what I’m currently working on and will be putting a PR up shortly.
Michael Crosby
… On May 26, 2020, at 6:53 PM, Derek McGowan ***@***.***> wrote:
Merged #1487 into master.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
thanks for taking this on
This is a carry of #1246.
We are splitting up the work and I will be carrying this PR to merge.