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.

Sign up for GitHub

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

Jump to bottom

added fluentd-elastisearch addon #1

Merged
merged 3 commits into from
Jun 10, 2021
Merged

added fluentd-elastisearch addon #1

merged 3 commits into from
Jun 10, 2021

Conversation

monotek
Copy link
Member

@monotek monotek commented Apr 12, 2021 •
edited

Signed-off-by: André Bauer monotek23@gmail.com

Todo / open for discussion if this should be part of this pr:

  • add github action ci
  • publish docker image to some registry controlled by k/k community members (currently quay.io is used)
  • maybe add me and @coffeepac to the ownersfile / sig instrumentation so we can maintain the addon again?

@monotek
added fluentd-elastisearch addon
467657f 
Signed-off-by: André Bauer <monotek23@gmail.com>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 12, 2021
@k8s-ci-robot
Copy link
Contributor

Welcome @monotek!

It looks like this is your first PR to kubernetes-sigs/instrumentation-addons 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/instrumentation-addons has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. labels Apr 12, 2021
This was referenced Apr 12, 2021
Closed
Closed
logicalhan
logicalhan approved these changes May 13, 2021
View reviewed changes
Copy link
Contributor

@logicalhan logicalhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 13, 2021
@ehashman ehashman mentioned this pull request May 13, 2021
Merged
ehashman
ehashman approved these changes May 13, 2021
View reviewed changes
Copy link
Contributor

@ehashman ehashman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

ehashman
ehashman reviewed May 13, 2021
View reviewed changes
fluentd-elasticsearch/OWNERS Outdated
Comment on lines 6 to 8
- sig-instrumentation-approvers
reviewers:
- sig-instrumentation-reviewers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will need to define an OWNERS_ALIASES for these in the root of the repo for this to work.

See e.g. https://github.com/kubernetes-sigs/prometheus-adapter/blob/master/OWNERS_ALIASES

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll need #3 approved/merged first. then we can run /verify-owners here and it'll get sorted.

Copy link
Member Author

@monotek monotek May 14, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If added the OWNERS_ALIAS file here too, as in #2

If we only need it in the repo root, as suggested by @ehashman, i'll remove it again.

@monotek
added OWNERS_ALIAS file
73ebc8a 
Signed-off-by: André Bauer <monotek23@gmail.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 14, 2021
@monotek
Copy link
Member Author

monotek commented May 14, 2021

Note to myself: add changes from kubernetes/kubernetes#100169

@monotek monotek mentioned this pull request May 14, 2021
Merged
@monotek
Copy link
Member Author

monotek commented May 14, 2021

/verify-owners

1 similar comment
@ehashman
Copy link
Contributor

/verify-owners

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label May 14, 2021
@ehashman
Copy link
Contributor

There we go. File wasn't named correctly. I fixed it in 4726f83

ehashman
ehashman suggested changes May 14, 2021
View reviewed changes
fluentd-elasticsearch/OWNERS_ALIAS Outdated Show resolved Hide resolved
@ehashman
Copy link
Contributor

/hold

to remove the redundant OWNERS_ALIAS file

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 14, 2021
@monotek
remove aliases file and add changes from kubernetes#100169
9adc01e 
Signed-off-by: André Bauer <monotek23@gmail.com>
@monotek
Copy link
Member Author

monotek commented May 18, 2021

@ehashman
OWNERS_ALIAS removed and updates from kubernetes/kubernetes#100169 added.

@monotek monotek requested a review from ehashman May 20, 2021 13:50
@coffeepac coffeepac mentioned this pull request Jun 4, 2021
Closed
@monotek
Copy link
Member Author

monotek commented Jun 5, 2021

@ehashman
ping :)

ehashman
ehashman reviewed Jun 10, 2021
View reviewed changes
Copy link
Contributor

@ehashman ehashman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/hold cancel
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jun 10, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ehashman, logicalhan, monotek

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:
  • OWNERS [ehashman,logicalhan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 05d5960 into kubernetes-sigs:master Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coffeepac coffeepac coffeepac left review comments

@ehashman ehashman ehashman requested changes

@logicalhan logicalhan logicalhan approved these changes

@brancz brancz Awaiting requested review from brancz

Assignees

@logicalhan logicalhan

@ehashman ehashman

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move fluentd-elasticsearch out of k/k
5 participants
@monotek @k8s-ci-robot @ehashman @coffeepac @logicalhan