*: add CRI-O handler by runcom · Pull Request #1741 · google/cadvisor

runcom · 2017-08-31T16:10:36Z

This patch adds native support for the CRI-O runtime to cadvisor.
Tested by integrating it with kubernetes and verified the necessary stats are now returned properly.

/cc @derekwaynecarr @mrunalp

Signed-off-by: Antonio Murdaca runcom@redhat.com

runcom · 2017-08-31T16:16:57Z

failure is because there's no CRI-O in the CI to test this with:

W0831 16:13:03.580] W0831 16:13:03.498099    2569 manager.go:166] unable to connect to CRI-O api service: Get http://localhost:7373/info: dial tcp 127.0.0.1:7373: getsockopt: connection refused

runcom · 2017-08-31T16:30:04Z

/retest

derekwaynecarr · 2017-08-31T17:30:27Z

@dashpole @dchen1107 -- this is the follow-up from our discussion in this week's sig-node. i can handle primary review on this.

derekwaynecarr · 2017-08-31T17:30:36Z

fyi @sjenning

runcom · 2017-08-31T20:13:49Z

failure doesn't seem related (?)

dashpole · 2017-08-31T20:25:57Z

#1742 builds without error. I havent seen this before, so it is likely something with this PR.

sjenning · 2017-08-31T20:34:41Z

Looks similiar to #1481 again ☹️ Maybe my netgo flag PR didn't fix it.

sjenning · 2017-08-31T20:34:52Z

/retest

runcom · 2017-09-01T14:02:50Z

Updated to make calls on the CRI-O socket

derekwaynecarr

Getting close!

derekwaynecarr · 2017-09-02T00:57:27Z

container/crio/client.go

+)
+
+const (
+	crioSocket            = "/var/run/crio.sock"


Can you make this public? I would want to reference it in kubelet

derekwaynecarr · 2017-09-02T00:59:14Z

container/crio/factory.go

+// The namespace under which crio aliases are unique.
+const CrioNamespace = "crio"
+
+// Regexp that identifies docker cgroups, containers started with


Nit from my port. s/docker/cri-o

derekwaynecarr · 2017-09-02T01:03:01Z

container/crio/factory.go

+
+// crio handles all containers under /crio
+func (self *crioFactory) CanHandleAndAccept(name string) (bool, bool, error) {
+	glog.Infof("CRIO CAN HANDLE AND ACCEPT: %v", name)


Cleanup logging

derekwaynecarr · 2017-09-02T01:03:17Z

container/crio/factory.go

+	if !strings.HasPrefix(path.Base(name), CrioNamespace) {
+		return false, false, nil
+	}
+	// if the container is not associated with docker, we can't handle it or accept it.


s/docker/crio

derekwaynecarr · 2017-09-02T01:04:01Z

container/crio/factory.go

+		return false, false, nil
+	}
+	glog.Infof("CRIO HANDLE AND ACCEPT: %v", name)
+	// TODO should we call equivalent of a crio info to be sure its really ours


I know I wrote this comment. I would prefer we do not do this for perf

derekwaynecarr · 2017-09-02T01:07:42Z

container/crio/handler.go

+	// Manager of this container's cgroups.
+	cgroupManager cgroups.Manager
+
+	// the docker storage driver


derekwaynecarr · 2017-09-02T01:12:41Z

manager/manager.go

 		},
-		RktPath: rktPath,
+		RktPath:  rktPath,
+		CrioPath: crioInfo.StorageRoot,


I think we should have this be a struct similar to docker

runcom · 2017-09-02T09:35:23Z

@derekwaynecarr addressed your comments

runcom · 2017-09-04T16:13:27Z

removing WIP label, I think it's fully ready for review (added some unit tests also):

kubectl get --raw /api/v1/nodes/127.0.0.1/proxy/stats/summary

{
  "node": {
   "nodeName": "127.0.0.1",
   "startTime": "2017-09-04T11:40:43Z",
   "cpu": {
    "time": "2017-09-04T16:13:10Z",
    "usageNanoCores": 454768832,
    "usageCoreNanoSeconds": 4424417531376
   },
   "memory": {
    "time": "2017-09-04T16:13:10Z",
    "availableBytes": 11485138944,
    "usageBytes": 10475511808,
    "workingSetBytes": 9253203968,
    "rssBytes": 3624992768,
    "pageFaults": 61345034,
    "majorPageFaults": 14271
   },
   "fs": {
    "time": "2017-09-04T16:13:10Z",
    "availableBytes": 13987569664,
    "capacityBytes": 52710469632,
    "usedBytes": 36021768192,
    "inodesFree": 2681895,
    "inodes": 3276800,
    "inodesUsed": 594905
   },
   "runtime": {
    "imageFs": {
     "time": "2017-09-04T16:13:10Z",
     "availableBytes": 13987569664,
     "capacityBytes": 52710469632,
     "usedBytes": 560776649,
     "inodesFree": 2681895,
     "inodes": 3276800,
     "inodesUsed": 594905
    }
   }
  },
  "pods": [
   {
    "podRef": {
     "name": "kube-dns-4124969034-6gqh6",
     "namespace": "kube-system",
     "uid": "fc25928b-918a-11e7-b392-507b9d4141fa"
    },
    "startTime": "2017-09-04T16:06:33Z",
    "containers": [
     {
      "name": "sidecar",
      "startTime": "2017-09-04T16:06:34Z",
      "cpu": {
       "time": "2017-09-04T16:13:09Z",
       "usageNanoCores": 1040646,
       "usageCoreNanoSeconds": 627473919
      },
      "memory": {
       "time": "2017-09-04T16:13:09Z",
       "usageBytes": 11571200,
       "workingSetBytes": 11444224,
       "rssBytes": 10747904,
       "pageFaults": 4191,
       "majorPageFaults": 1
      },
      "rootfs": {
       "time": "2017-09-04T16:13:09Z",
       "availableBytes": 13987569664,
       "capacityBytes": 52710469632,
       "usedBytes": 42471424,
       "inodesFree": 2681895,
       "inodes": 3276800,
       "inodesUsed": 14
      },
      "logs": {
       "time": "2017-09-04T16:13:09Z",
       "availableBytes": 13987569664,
       "capacityBytes": 52710469632,
       "usedBytes": 20480,
       "inodesFree": 2681895,
       "inodes": 3276800,
       "inodesUsed": 594905
      },
      "userDefinedMetrics": null
     },
     {
      "name": "dnsmasq",
      "startTime": "2017-09-04T16:06:34Z",
      "cpu": {
       "time": "2017-09-04T16:13:07Z",
       "usageNanoCores": 848354,
       "usageCoreNanoSeconds": 169743690
      },
      "memory": {
       "time": "2017-09-04T16:13:07Z",
       "usageBytes": 7319552,
       "workingSetBytes": 7315456,
       "rssBytes": 6520832,
       "pageFaults": 2759,
       "majorPageFaults": 0
      },
      "rootfs": {
       "time": "2017-09-04T16:13:07Z",
       "availableBytes": 13987569664,
       "capacityBytes": 52710469632,
       "usedBytes": 42090496,
       "inodesFree": 2681895,
       "inodes": 3276800,
       "inodesUsed": 15
      },
      "logs": {
       "time": "2017-09-04T16:13:07Z",
       "availableBytes": 13987569664,
       "capacityBytes": 52710469632,
       "usedBytes": 20480,
       "inodesFree": 2681895,
       "inodes": 3276800,
       "inodesUsed": 594905
      },
      "userDefinedMetrics": null
     },
     {
      "name": "kubedns",
      "startTime": "2017-09-04T16:06:33Z",
      "cpu": {
       "time": "2017-09-04T16:13:10Z",
       "usageNanoCores": 609739,
       "usageCoreNanoSeconds": 348014134
      },
      "memory": {
       "time": "2017-09-04T16:13:10Z",
       "availableBytes": 170975232,
       "usageBytes": 7282688,
       "workingSetBytes": 7282688,
       "rssBytes": 6557696,
       "pageFaults": 2524,
       "majorPageFaults": 0
      },
      "rootfs": {
       "time": "2017-09-04T16:13:10Z",
       "availableBytes": 13987569664,
       "capacityBytes": 52710469632,
       "usedBytes": 50057216,
       "inodesFree": 2681895,
       "inodes": 3276800,
       "inodesUsed": 15
      },
      "logs": {
       "time": "2017-09-04T16:13:10Z",
       "availableBytes": 13987569664,
       "capacityBytes": 52710469632,
       "usedBytes": 20480,
       "inodesFree": 2681895,
       "inodes": 3276800,
       "inodesUsed": 594905
      },
      "userDefinedMetrics": null
     }
    ],
    "network": {
     "time": "2017-09-04T16:13:03Z",
     "rxBytes": 290526,
     "rxErrors": 0,
     "txBytes": 266470,
     "txErrors": 0
    },
    "volume": [
     {
      "time": "2017-09-04T16:07:15Z",
      "availableBytes": 10369159168,
      "capacityBytes": 10369171456,
      "usedBytes": 12288,
      "inodesFree": 2531527,
      "inodes": 2531536,
      "inodesUsed": 9,
      "name": "kube-dns-token-8b24g"
     }
    ]
   }
  ]
 }

runcom · 2017-09-05T09:41:52Z

pretty sure tests failure is unrelated. Added some unit tests also.

derekwaynecarr · 2017-09-05T14:54:39Z

/test pull-cadvisor-e2e

derekwaynecarr · 2017-09-05T14:58:33Z

@runcom -- looks like a gofmt error on handler_test.go i think.

Signed-off-by: Antonio Murdaca <runcom@redhat.com>

runcom · 2017-09-05T15:02:09Z

@runcom -- looks like a gofmt error on handler_test.go i think.

@derekwaynecarr fixed

derekwaynecarr · 2017-09-06T18:15:04Z

container/crio/client.go

+	// No need for compression in local communications.
+	tr.DisableCompression = true
+	tr.Dial = func(_, _ string) (net.Conn, error) {
+		return net.DialTimeout(proto, addr, 32*time.Second)


can this be shorter? rkt is 2s

docker has the exact same value when using socket connection

ok, thanks for the detail.

derekwaynecarr · 2017-09-06T18:49:52Z

@runcom - just the one question on timeout looking rather high. once addressed, this is LGTM

derekwaynecarr · 2017-09-06T19:48:10Z

LGTM

Automatic merge from submit-queue (batch tested with PRs 51728, 49202) Enable CRI-O stats from cAdvisor **What this PR does / why we need it**: cAdvisor may support multiple container runtimes (docker, rkt, cri-o, systemd, etc.) As long as the kubelet continues to run cAdvisor, runtimes with native cAdvisor support may not want to run multiple monitoring agents to avoid performance regression in production. Pending kubelet running a more light-weight monitoring solution, this PR allows remote runtimes to have their stats pulled from cAdvisor when cAdvisor is registered stats provider by introspection of the runtime endpoint. See issue kubernetes#51798 **Special notes for your reviewer**: cAdvisor will be bumped to pick up google/cadvisor#1741 At that time, CRI-O will support fetching stats from cAdvisor. **Release note**: ```release-note NONE ```

@derekwaynecarr

Automatic merge from submit-queue cadvisor/runc updates Support CRI-O: google/cadvisor#1741 Fix memory stats: google/cadvisor#1728 opencontainers/runc#1378 @derekwaynecarr

runcom force-pushed the crio-handler branch from 34f7b62 to f7dc1b6 Compare August 31, 2017 16:19

derekwaynecarr self-assigned this Aug 31, 2017

derekwaynecarr mentioned this pull request Aug 31, 2017

Enable CRI-O stats from cAdvisor kubernetes/kubernetes#51728

Merged

runcom force-pushed the crio-handler branch from f7dc1b6 to 2bcd3d9 Compare August 31, 2017 18:53

dashpole mentioned this pull request Aug 31, 2017

See if builds are broken #1742

Closed

runcom force-pushed the crio-handler branch from 2bcd3d9 to 5383840 Compare September 1, 2017 13:28

runcom mentioned this pull request Sep 1, 2017

serve grpc and http on the same socket cri-o/cri-o#831

Merged

runcom force-pushed the crio-handler branch 4 times, most recently from c0cfa4b to e70a9de Compare September 2, 2017 00:13

derekwaynecarr suggested changes Sep 2, 2017

View reviewed changes

runcom force-pushed the crio-handler branch 2 times, most recently from 559cbac to 46c9379 Compare September 2, 2017 09:34

runcom force-pushed the crio-handler branch 2 times, most recently from cde0f22 to 3c40183 Compare September 4, 2017 16:11

runcom changed the title ~~[WIP] *: add CRI-O handler~~ *: add CRI-O handler Sep 4, 2017

runcom force-pushed the crio-handler branch 2 times, most recently from 0da90e4 to fe9ddf3 Compare September 5, 2017 09:34

runcom force-pushed the crio-handler branch from fe9ddf3 to 77e4fd3 Compare September 5, 2017 09:38

*: add CRI-O handler

4b002b3

Signed-off-by: Antonio Murdaca <runcom@redhat.com>

runcom force-pushed the crio-handler branch from 77e4fd3 to 4b002b3 Compare September 5, 2017 15:02

derekwaynecarr reviewed Sep 6, 2017

View reviewed changes

derekwaynecarr approved these changes Sep 6, 2017

View reviewed changes

derekwaynecarr merged commit e8dbd50 into google:master Sep 6, 2017

runcom deleted the crio-handler branch September 6, 2017 19:49

runcom mentioned this pull request Sep 8, 2017

Failed to check if disk space is available for the runtime cri-o/cri-o#790

Closed

sjenning mentioned this pull request Sep 18, 2017

cadvisor/runc updates openshift/origin#16419

Merged

Conversation

runcom commented Aug 31, 2017 • edited by derekwaynecarr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

runcom commented Aug 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

runcom commented Aug 31, 2017

Uh oh!

derekwaynecarr commented Aug 31, 2017

Uh oh!

derekwaynecarr commented Aug 31, 2017

Uh oh!

runcom commented Aug 31, 2017

Uh oh!

dashpole commented Aug 31, 2017

Uh oh!

sjenning commented Aug 31, 2017

Uh oh!

sjenning commented Aug 31, 2017

Uh oh!

runcom commented Sep 1, 2017

Uh oh!

derekwaynecarr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

runcom commented Sep 2, 2017

Uh oh!

runcom commented Sep 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

runcom commented Sep 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

derekwaynecarr commented Sep 5, 2017

Uh oh!

derekwaynecarr commented Sep 5, 2017

Uh oh!

runcom commented Sep 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

derekwaynecarr commented Sep 6, 2017

Uh oh!

derekwaynecarr commented Sep 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

runcom commented Aug 31, 2017 •

edited by derekwaynecarr

Loading

runcom commented Aug 31, 2017 •

edited

Loading

runcom commented Sep 4, 2017 •

edited

Loading

runcom commented Sep 5, 2017 •

edited

Loading