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

Update to latest cadvisor - cleanup mesos/rkt #76291

Merged

Conversation

@dims
Copy link
Member

dims commented Apr 8, 2019

Change-Id: I2aa91b4dd1f421fdf72623e5dc94d4f304d9b76b

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

Mainly to bring in the code from the following PRs:

This cleans up rkt/mesos and their dependencies

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

The kubelet's /spec endpoint no longer provides cloud provider information (cloud_provider, instance_type, instance_id). 
@dims

This comment has been minimized.

Copy link
Member Author

dims commented Apr 8, 2019

/sig node
/priority important-soon

@dims dims force-pushed the dims:update-to-latest-cadvisor-remove-mesos-rkt branch from 21c3968 to c2ceccb Apr 8, 2019

@k8s-ci-robot k8s-ci-robot added size/L and removed size/XXL labels Apr 8, 2019

@dims dims force-pushed the dims:update-to-latest-cadvisor-remove-mesos-rkt branch from c2ceccb to bd62e8e Apr 9, 2019

@dims dims changed the title [WIP] Update to latest cadvisor - cleanup mesos/rkt Update to latest cadvisor - cleanup mesos/rkt Apr 9, 2019

@dashpole

This comment has been minimized.

Copy link
Contributor

dashpole commented Apr 9, 2019

/lgtm

@@ -89,7 +103,7 @@ func New(imageFsInfoProvider ImageFsInfoProvider, rootPath string, usingLegacySt
// collect metrics for all cgroups
rawContainerCgroupPathPrefixWhiteList := []string{"/"}
// Create and start the cAdvisor container manager.
m, err := manager.New(memory.New(statsCacheDuration, nil), sysFs, maxHousekeepingInterval, allowDynamicHousekeeping, includedMetrics, http.DefaultClient, rawContainerCgroupPathPrefixWhiteList)
m, err := newCadvisor(memory.New(statsCacheDuration, nil), sysFs, maxHousekeepingInterval, allowDynamicHousekeeping, includedMetrics, http.DefaultClient, rawContainerCgroupPathPrefixWhiteList)

This comment has been minimized.

Copy link
@liggitt

liggitt Apr 9, 2019

Member

talked with @dims about an approach that would avoid needing to change this signature and keep us (and all callers) from having to copy out initialization code

will open a PR to cadvisor and sync up there

This comment has been minimized.

Copy link
@liggitt

liggitt Apr 10, 2019

Member

opened google/cadvisor#2217, keeping all the collector initialization contained to manager.New() ... testing with CI in #76355

@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Apr 9, 2019

/hold

per #76291 (comment)

@dims

This comment has been minimized.

Copy link
Member Author

dims commented Apr 9, 2019

Thanks @liggitt

@dims dims force-pushed the dims:update-to-latest-cadvisor-remove-mesos-rkt branch from 694047f to 41eb2ea Apr 11, 2019

@k8s-ci-robot k8s-ci-robot added size/M and removed lgtm size/L labels Apr 11, 2019

@dims dims force-pushed the dims:update-to-latest-cadvisor-remove-mesos-rkt branch from 41eb2ea to 94f3e46 Apr 11, 2019

Update to latest cadvisor - cleanup mesos/rkt
Change-Id: Ib5ae0cb13b93f8c87bb74e3ba33040df5f3d6a6f

@dims dims force-pushed the dims:update-to-latest-cadvisor-remove-mesos-rkt branch from 94f3e46 to 6993654 Apr 11, 2019

Adjust our use of cadvisor to pull in just runtimes we need
- We drop mesos and rkt support in cadvisor

Change-Id: Id4dcafdecc97802b2def95a572e67842e7507f7b

@dims dims force-pushed the dims:update-to-latest-cadvisor-remove-mesos-rkt branch from 6993654 to 1707144 Apr 11, 2019

@dims

This comment has been minimized.

Copy link
Member Author

dims commented Apr 11, 2019

/test pull-kubernetes-dependencies

@fedebongio

This comment has been minimized.

Copy link
Contributor

fedebongio commented Apr 11, 2019

/remove-sig api-machinery

@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Apr 11, 2019

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Apr 11, 2019

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Apr 11, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, liggitt

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@liggitt

This comment has been minimized.

Copy link
Member

liggitt commented Apr 11, 2019

/hold cancel

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Apr 12, 2019

@dims: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-godeps 94f3e46 link /test pull-kubernetes-godeps

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@fejta-bot

This comment has been minimized.

Copy link

fejta-bot commented Apr 12, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit 78f571d into kubernetes:master Apr 12, 2019

18 checks passed

cla/linuxfoundation dims authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Job succeeded.
Details
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.