Skip to content

fix systemd slice expansion so that it could be consumed by cAdvisor#1722

Merged
crosbymichael merged 1 commit intoopencontainers:masterfrom
ravisantoshgudimetla:fix-systemd-path
Feb 20, 2018
Merged

fix systemd slice expansion so that it could be consumed by cAdvisor#1722
crosbymichael merged 1 commit intoopencontainers:masterfrom
ravisantoshgudimetla:fix-systemd-path

Conversation

@ravisantoshgudimetla
Copy link
Copy Markdown
Contributor

@ravisantoshgudimetla ravisantoshgudimetla commented Feb 19, 2018

As of now systemd slice expansion would result in strings with a suffix "/" but we want "/" to be prefixed in kubernetes so that cAdvisor can read stats.

The only call to ExpandSlice function in runc seems to be happening from

https://github.com/opencontainers/runc/blob/master/libcontainer/cgroups/systemd/apply_systemd.go#L449

which uses filepath.Join, so shouldn't cause a behavioural change.

xref: kubernetes/kubernetes#59993

/cc @derekwaynecarr @sjenning

Signed-off-by: ravisantoshgudimetla ravisantoshgudimetla@gmail.com

Signed-off-by: ravisantoshgudimetla <ravisantoshgudimetla@gmail.com>
@sjenning
Copy link
Copy Markdown
Contributor

Hold on this. It doesn't seem to address the upstream issue kubernetes/kubernetes#59993 (comment)

@sjenning
Copy link
Copy Markdown
Contributor

@sjenning
Copy link
Copy Markdown
Contributor

cc @mrunalp

@sjenning
Copy link
Copy Markdown
Contributor

not that I have any privileges but LGTM

@cyphar
Copy link
Copy Markdown
Member

cyphar commented Feb 20, 2018

LGTM.

Approved with PullApprove

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Feb 20, 2018

LGTM

Approved with PullApprove

@crosbymichael crosbymichael merged commit 595bea0 into opencontainers:master Feb 20, 2018
@ravisantoshgudimetla ravisantoshgudimetla deleted the fix-systemd-path branch February 24, 2018 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants