fix: prometheus input plugin run outside k8s cluster err #9960

yahaa · 2021-10-20T05:21:57Z

Required for all PRs:

Updated associated README.md.
Wrote appropriate unit tests.
Pull request title or commits are in conventional commit format

resolves #

fix prometheus input plugin run outside k8s cluster err.

telegraf-tiger · 2021-10-20T05:22:03Z

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

telegraf-tiger · 2021-10-20T05:27:56Z

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

yahaa · 2021-10-20T07:24:47Z

!signed-cla

sspaink · 2021-10-20T19:15:22Z

@yahaa thanks for the pull request, can you add more detail on the error this is trying to resolve?
I noticed you resolved the issue where watcher is being called before the error is checked, so that is great.

The CI is failing because github.com/imdario/mergo and github.com/spf13/pflag aren't in the right place in the LICENSE_OF_DEPENDENCIES.md it is kind of picky, it needs to be in alphabetical order.

yahaa · 2021-10-21T02:36:22Z

@yahaa thanks for the pull request, can you add more detail on the error this is trying to resolve? I noticed you resolved the issue where watcher is being called before the error is checked, so that is great.

The CI is failing because github.com/imdario/mergo and github.com/spf13/pflag aren't in the right place in the LICENSE_OF_DEPENDENCIES.md it is kind of picky, it needs to be in alphabetical order.

The first one, when we run telegra in outside of a kubernetes cluster the line config, err := rest.InClusterConfig() will be always get an error.

The second one, the method of getting rest.Config instance in loadClient function will be cause kubernetes client has error materURL, and this error can be reproduced by using the following UT:

// add this UT into plugins/inputs/prometheus/kubernetes_test.go
func TestLoadClient(t *testing.T) {
	kubeconfig := "/path_to_your_kubeconfig"
	client, err := loadClient(kubeconfig)
	if err != nil {
		t.Errorf("got err: %v", err)
		return
	}

	watcher, err := client.CoreV1().Pods("kube-system").Watch(context.TODO(), metav1.ListOptions{
		LabelSelector: "app=debug",
	})
	if err != nil {
		t.Errorf("watch pod got err: %v", err)
		return
	}
	defer watcher.Stop()
}

this UT can got watch pod got err: Get "http://localhost/api/v1/namespaces/kube-system/pods?labelSelector=app%3Ddebug&watch=true": dial tcp 127.0.0.1:80: connect: connection refused error.

telegraf-tiger · 2021-10-21T03:06:20Z

📦 Looks like new artifacts were built from this PR.

Expand this list to get them here! 🐯

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armel.deb	armv6hl.rpm	freebsd_armv7.tar.gz
armhf.deb	i386.rpm	freebsd_i386.tar.gz
i386.deb	ppc64le.rpm	linux_amd64.tar.gz
mips.deb	s390x.rpm	linux_arm64.tar.gz
mipsel.deb	x86_64.rpm	linux_armel.tar.gz
ppc64el.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_s390x.tar.gz
		static_linux_amd64.tar.gz

yahaa · 2021-10-22T06:28:53Z

@sspaink Please help me review this PR, thanks!

powersj

Hi,

Thanks for this PR! If you are still interested in landing this, can you please rebase this on master, resolve the conflicts, and answer my question about the check-deps changes?

Thanks!

powersj · 2021-11-10T20:32:16Z

scripts/check-deps.sh


-grep '^-' docs/LICENSE_OF_DEPENDENCIES.md | grep -v github.com/DataDog/datadog-agent | cut -f 2 -d' ' > "${tmpdir}/LICENSE_OF_DEPENDENCIES.md"
+grep '^-' docs/LICENSE_OF_DEPENDENCIES.md | grep -v github.com/DataDog/datadog-agent | cut -f 2 -d' ' | sort -n > "${tmpdir}/LICENSE_OF_DEPENDENCIES.md"


Why are these changes required for this PR? Seems odd to sort numerically.

Ok, Thanks for your feedback, i have rebase and resolve the conflicts.

It is difficult to add github.com/imdario/mergo and github.com/spf13/pflag in the right place in the LICENSE_OF_DEPENDENCIES.md because we need to ensure it in alphabetical order.

Hmm go mod tidy did not handle that for you? I am still seeing a conflict.

@yahaa I'd prefer if we kept the dependency licenses in alphabetical order opposed to numerical, would you mind removing this change? We could always consider this in a separate pull request.

@yahaa I'd prefer if we kept the dependency licenses in alphabetical order opposed to numerical, would you mind removing this change? We could always consider this in a separate pull request.

Sorry, forgot to reply you. I also think a single pr would be better.

telegraf-tiger · 2021-11-12T09:28:33Z

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1% increase)

📦 Looks like new artifacts were built from this PR.

Expand this list to get them here ! 🐯

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armel.deb	armv6hl.rpm	freebsd_armv7.tar.gz
armhf.deb	i386.rpm	freebsd_i386.tar.gz
i386.deb	ppc64le.rpm	linux_amd64.tar.gz
mips.deb	s390x.rpm	linux_arm64.tar.gz
mipsel.deb	x86_64.rpm	linux_armel.tar.gz
ppc64el.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_s390x.tar.gz
		static_linux_amd64.tar.gz

telegraf-tiger bot added the fix pr to fix corresponding bug label Oct 20, 2021

yahaa changed the title ~~fix prometheus input plugin run outside k8s cluster err~~ fix: prometheus input plugin run outside k8s cluster err Oct 20, 2021

yahaa force-pushed the fix-prometheus-input-plugin branch 2 times, most recently from bc0f232 to e07fb44 Compare October 20, 2021 08:30

sspaink mentioned this pull request Oct 20, 2021

fix(prometheus): move watcher error check #9965

Closed

yahaa force-pushed the fix-prometheus-input-plugin branch from e07fb44 to 5addf2b Compare October 21, 2021 02:45

powersj suggested changes Nov 10, 2021

View reviewed changes

yahaa force-pushed the fix-prometheus-input-plugin branch from 5addf2b to cfd8046 Compare November 12, 2021 08:52

fix: prometheus input plugin run outside k8s cluster err

598438c

yahaa force-pushed the fix-prometheus-input-plugin branch from cfd8046 to 598438c Compare November 12, 2021 09:07

yahaa requested a review from powersj November 12, 2021 09:21

yahaa force-pushed the fix-prometheus-input-plugin branch from 88129a5 to 598438c Compare November 16, 2021 03:39

sspaink added the waiting for response waiting for response from contributor label Jun 21, 2022

yahaa closed this Jul 2, 2022

yahaa mentioned this pull request Aug 15, 2022

fix: prometheus input plugin run outside k8s cluster error #11673

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: prometheus input plugin run outside k8s cluster err #9960

fix: prometheus input plugin run outside k8s cluster err #9960

yahaa commented Oct 20, 2021 •

edited

Loading

telegraf-tiger bot commented Oct 20, 2021

telegraf-tiger bot commented Oct 20, 2021

yahaa commented Oct 20, 2021

sspaink commented Oct 20, 2021

yahaa commented Oct 21, 2021 •

edited

Loading

telegraf-tiger bot commented Oct 21, 2021

Artifact URLs

yahaa commented Oct 22, 2021

powersj left a comment

powersj Nov 10, 2021

yahaa Nov 12, 2021 •

edited

Loading

powersj Nov 15, 2021

sspaink Jun 21, 2022

yahaa Aug 15, 2022

telegraf-tiger bot commented Nov 12, 2021

Artifact URLs


		grep '^-' docs/LICENSE_OF_DEPENDENCIES.md \| grep -v github.com/DataDog/datadog-agent \| cut -f 2 -d' ' > "${tmpdir}/LICENSE_OF_DEPENDENCIES.md"
		grep '^-' docs/LICENSE_OF_DEPENDENCIES.md \| grep -v github.com/DataDog/datadog-agent \| cut -f 2 -d' ' \| sort -n > "${tmpdir}/LICENSE_OF_DEPENDENCIES.md"

fix: prometheus input plugin run outside k8s cluster err #9960

fix: prometheus input plugin run outside k8s cluster err #9960

Conversation

yahaa commented Oct 20, 2021 • edited Loading

Required for all PRs:

telegraf-tiger bot commented Oct 20, 2021

telegraf-tiger bot commented Oct 20, 2021

yahaa commented Oct 20, 2021

sspaink commented Oct 20, 2021

yahaa commented Oct 21, 2021 • edited Loading

telegraf-tiger bot commented Oct 21, 2021

Artifact URLs

yahaa commented Oct 22, 2021

powersj left a comment

Choose a reason for hiding this comment

powersj Nov 10, 2021

Choose a reason for hiding this comment

yahaa Nov 12, 2021 • edited Loading

Choose a reason for hiding this comment

powersj Nov 15, 2021

Choose a reason for hiding this comment

sspaink Jun 21, 2022

Choose a reason for hiding this comment

yahaa Aug 15, 2022

Choose a reason for hiding this comment

telegraf-tiger bot commented Nov 12, 2021

Artifact URLs

yahaa commented Oct 20, 2021 •

edited

Loading

yahaa commented Oct 21, 2021 •

edited

Loading

yahaa Nov 12, 2021 •

edited

Loading