Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

Remove unused features (socket activation and KSM) #177

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jodh-intel
Copy link
Contributor

No description provided.

@jodh-intel
Copy link
Contributor Author

Note that this cannot realistically land until we a stand-alone daemon to handle KSM, hence the dnm label.

See:

@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@jodh-intel
Copy link
Contributor Author

The tests are failing due to https://github.com/clearcontainers/tests/blob/master/.ci/install_proxy.sh#L34..L35 which is trying to modify the proxy's service file (which has been removed by this PR).

Once we have a KSM daemon, we can then:

  • Raise a tests PR to tweak install_proxy.sh.
  • Re-run the CI tests for this PR.

chavafg added a commit to chavafg/tests-cc that referenced this pull request Dec 6, 2017
Now that the cc-proxy will not be a service, we need to use
option -t in journalctl to get the logs from the proxy.
Relates to:
clearcontainers/runtime#835
clearcontainers/proxy#177

Fixes: clearcontainers#775

Signed-off-by: Salvador Fuentes <[email protected]>
chavafg added a commit to chavafg/tests-cc that referenced this pull request Dec 6, 2017
Now that the cc-proxy will not be a service, we need to use
option -t in journalctl to get the logs from the proxy.
Relates to:
clearcontainers/runtime#835
clearcontainers/proxy#177

Fixes: clearcontainers#775

Signed-off-by: Salvador Fuentes <[email protected]>
@clearcontainersbot
Copy link

kubernetes qa-failed 👎

The proxy should not (cannot) handle KSM now that an instance is being
launched per pod.

Partially fixes clearcontainers#176.

Signed-off-by: James O. D. Hunt <[email protected]>
Socket activation is no longer required since a proxy instance
is launched by the runtime for each pod.

Fixes clearcontainers#176.

Signed-off-by: James O. D. Hunt <[email protected]>
@jodh-intel jodh-intel force-pushed the remove-unused-features branch from 7e120ec to e3ab55e Compare December 14, 2017 14:45
@clearcontainersbot
Copy link

kubernetes qa-failed 👎

1 similar comment
@clearcontainersbot
Copy link

kubernetes qa-failed 👎

@jodh-intel
Copy link
Contributor Author

Blocked on clearcontainers/tests#913.

We cannot remove the KSM functionality from the proxy until the issue above is resolved since the tests currently install the proxy "manually" using:

(Note that user systems would not be affected by removing the functionality since they have the cc-ksm-throttler package installed already).

@jodh-intel
Copy link
Contributor Author

Hi @chavafg, @grahamwhaley - now that clearcontainers/tests#920 has landed, could you both confirm that I can continue with removing these features from the proxy?

@grahamwhaley
Copy link
Contributor

Let me check some stuff. afaict when we landed clearcontainers/tests#911 then the metrics CI got to do the right thing, but it then appears when we landed clearcontainers/tests#920 it might be back to being nobbled again (that is, KSM seems to be in effect even when we try to turn it off).
I'll have to go debug - which is made a fraction harder as the scripts don't generate any ack/nack/logs about what they are doing, so I have to go poke around the actual system... pls wait!

(oh, and apparently you need a rebase I think any how ;-) )

@jodh-intel
Copy link
Contributor Author

yep - I'll hold off rebasing until I get two confirmations :)

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

Successfully merging this pull request may close these issues.

3 participants