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

gcplogging driver MRPB for VM set #32930

Merged
merged 1 commit into from May 9, 2017

Conversation

Projects
None yet
5 participants
@gjaskiewicz
Contributor

gjaskiewicz commented Apr 29, 2017

Signed-off-by: Grzegorz Jaśkiewicz gj.jaskiewicz@gmail.com

- What I did

Added metadata about monitored resource type for GCP logging driver.

- How I did it

I used GCP logging SDK.

- How to verify it

Run a docker container using GCP logging driver on GCE VM and go to GCE VM logs, observe that logs are attributed to VM rather than being logged to global sink. Example to run a command could be
docker run --log-driver=gcplogs docker.io/tomcat

Before this change:
gcplogging-before

After this change:
gcplogging-after

- Description for the changelog
Add monitored resource type metadata for GCP logging driver

- A picture of a cute animal (not mandatory but encouraged)
hamster2

@tophj-ibm

This comment has been minimized.

Show comment
Hide comment
@tophj-ibm

tophj-ibm May 1, 2017

Contributor

Test failed because you need to gofmt the file:

21:10:47 These files are not properly gofmt'd:
21:10:47  - daemon/logger/gcplogs/gcplogging.go
21:10:47 
21:10:47 Please reformat the above files using "gofmt -s -w" and commit the result.
Contributor

tophj-ibm commented May 1, 2017

Test failed because you need to gofmt the file:

21:10:47 These files are not properly gofmt'd:
21:10:47  - daemon/logger/gcplogs/gcplogging.go
21:10:47 
21:10:47 Please reformat the above files using "gofmt -s -w" and commit the result.
@gjaskiewicz

This comment has been minimized.

Show comment
Hide comment
@gjaskiewicz

gjaskiewicz May 8, 2017

Contributor

@tophj-ibm done
@thaJeztah could you please assign a reviewer?

Contributor

gjaskiewicz commented May 8, 2017

@tophj-ibm done
@thaJeztah could you please assign a reviewer?

@thaJeztah

LGTM

can you squash the commits?

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah
Member

thaJeztah commented May 8, 2017

@vdemeester

LGTM 🐸

gcplogging driver MRPB set
Signed-off-by: Grzegorz Jaśkiewicz <gj.jaskiewicz@gmail.com>
@gjaskiewicz

This comment has been minimized.

Show comment
Hide comment
@gjaskiewicz

gjaskiewicz May 9, 2017

Contributor

@thaJeztah done
thanks for the review

Contributor

gjaskiewicz commented May 9, 2017

@thaJeztah done
thanks for the review

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah May 9, 2017

Member
12:35:29 ----------------------------------------------------------------------
12:35:29 FAIL: docker_api_stats_test.go:23: DockerSuite.TestAPIStatsNoStreamGetCpu
12:35:29 
12:35:29 docker_api_stats_test.go:60:
12:35:29     c.Assert(cpuPercent, check.Not(checker.Equals), 0.0, check.Commentf("docker stats with no-stream get cpu usage failed: was %v", cpuPercent))
12:35:29 ... obtained float64 = 0
12:35:29 ... expected float64 = 0
12:35:29 ... docker stats with no-stream get cpu usage failed: was 0
12:35:29 
12:35:31 
12:35:31 ----------------------------------------------------------------------

flaky test?

Member

thaJeztah commented May 9, 2017

12:35:29 ----------------------------------------------------------------------
12:35:29 FAIL: docker_api_stats_test.go:23: DockerSuite.TestAPIStatsNoStreamGetCpu
12:35:29 
12:35:29 docker_api_stats_test.go:60:
12:35:29     c.Assert(cpuPercent, check.Not(checker.Equals), 0.0, check.Commentf("docker stats with no-stream get cpu usage failed: was %v", cpuPercent))
12:35:29 ... obtained float64 = 0
12:35:29 ... expected float64 = 0
12:35:29 ... docker stats with no-stream get cpu usage failed: was 0
12:35:29 
12:35:31 
12:35:31 ----------------------------------------------------------------------

flaky test?

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah May 9, 2017

Member

Opened #33112 for the flaky test

Member

thaJeztah commented May 9, 2017

Opened #33112 for the flaky test

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah May 9, 2017

Member

All green now, merging.

Thanks @gjaskiewicz !

Member

thaJeztah commented May 9, 2017

All green now, merging.

Thanks @gjaskiewicz !

@thaJeztah thaJeztah merged commit 3a2d68a into moby:master May 9, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 33856 has succeeded
Details
janky Jenkins build Docker-PRs 42456 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 2794 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 13696 has succeeded
Details
z Jenkins build Docker-PRs-s390x 2553 has succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment