-
Notifications
You must be signed in to change notification settings - Fork 38.6k
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
Adjust Fluentd source format for Docker files to be JSON for Cloud Lo… #9687
Conversation
1c1beef
to
a8cc9ee
Compare
config change LGTM |
@@ -5,7 +5,7 @@ metadata: | |||
spec: | |||
containers: | |||
- name: fluentd-cloud-logging | |||
image: gcr.io/google_containers/fluentd-gcp:1.6 | |||
image: kubernetes/fluentd-gcp:1.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...but we should never commit this to the code base, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. After LGTM this should be changed to gcr.io/google_containers/fluentd-gcp:1.7
Does that make sense as a sane procedure for updating images?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to gcr.io/google_containers/fluentd-gcp:1.7
I can observe correctly ingested log data in the Cloud Logging console from a cluster made with this PR. |
I have now pushed the image to gcr.io.
|
GCE e2e build/test failed for commit e3c827c3100324bc9b5764f6b281dee9af2ebb45. |
GCE e2e build/test failed for commit a8cc9eea168dbd5f298b10f3feaf90c6baa87d51. |
GCE e2e build/test failed for commit b9b197e96b0622ed1d4660ce58a7c5520677b939. |
All: is this LGTM-ed enough to merge? |
# Rules for building the test image for deployment to Dockerhub with user kubernetes. | ||
|
||
kbuild: | ||
docker build -t kubernetes/fluentd-gcp:$(TAG) . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gcr.io/google_containers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likwise, this is for the test build of the image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh-- sorry, I missed the k.
LGTM after fixing gcr.io/google_containers :) |
I think this is ready to merge? |
Automatic merge failed. Can you check if rebase is needed? Thank you. |
Fetched and rebased from head, pushed. |
GCE e2e build/test passed for commit 33311b4. |
Adjust Fluentd source format for Docker files to be JSON for Cloud Lo…
…gging
@lavalamp because I have decided to diverge from the image update procedure followed elsewhere -- if we use the user kubernetes on Dockerhub then we don't need to use a Git hash -- right?
@mr-salty and @a-robinson for the config change