Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert mount-utils back to utils/mount #2726

Merged
merged 1 commit into from
Nov 11, 2020

Conversation

dims
Copy link
Collaborator

@dims dims commented Nov 11, 2020

@brahmaroutu we need to revert the change in #2687 because we are causing a recursion

  • mount-utils is in staging
  • k/k uses cadvisor
  • cadvisor uses the k8s.io/mount-utils

So this is not going to work. So switching cadvisor back to k8s.io/utils/mount

Signed-off-by: Davanum Srinivas [email protected]

@google-cla google-cla bot added the cla: yes label Nov 11, 2020
@dims dims requested a review from bobbypage November 11, 2020 03:48
@brahmaroutu
Copy link
Contributor

@dims Sorry if this caused recursion. We already moved utils/mount to staging. With cAdvisor using util/mount we will not be able to retire this package. Can you please explain why third party software cannot use staging modules?

@dims
Copy link
Collaborator Author

dims commented Nov 11, 2020

third party software can use staging modules IFF the third party software is not vendored back into k/k :)

@dims
Copy link
Collaborator Author

dims commented Nov 11, 2020

we have to live with k8s.io/utils/mount or copy that stuff over to cadvisor or find another library which is equivalent.

example : https://github.com/moby/sys/tree/master/mountinfo

But let's not rush into doing that right now, we can do in k8s 1.21 timeframe.

@brahmaroutu
Copy link
Contributor

Let us address this in 1.21 ASAP. Since 90% of the work to move utils/mount into staging is already done, I would like to make this work without maintaining multiple copies of the mount library and if possible, without retreating the changes made so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants