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.

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

Migrate from github.com/ericchiang/k8s to github.com/kubernetes/client-go #8937

Merged
merged 9 commits into from
Mar 17, 2021

Conversation

sspaink
Copy link
Contributor

@sspaink sspaink commented Mar 4, 2021

The Kubernetes client project ericchiang/k8s has been archived since 2019, looks like kubernetes/client-go is the replacement. Migrated the plugin kube_inventory and Prometheus to use the new client.

Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

🤝 ✅ CLA has been signed. Thank you!

Copy link
Contributor

@ivorybilled ivorybilled left a comment

Choose a reason for hiding this comment

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

Nice, makes some of the code cleaner too

@srebhan srebhan mentioned this pull request Mar 9, 2021
3 tasks
@sspaink sspaink added the waiting for response waiting for response from contributor label Mar 11, 2021
@sspaink sspaink removed the waiting for response waiting for response from contributor label Mar 17, 2021
@sspaink sspaink merged commit 79f5803 into influxdata:master Mar 17, 2021
@sspaink sspaink mentioned this pull request Apr 1, 2021
3 tasks
jblesener pushed a commit to jblesener/telegraf that referenced this pull request Apr 18, 2021
…t-go (influxdata#8937)

* new k8 client

* Make all tests pass

* Update licenses

* add timeout back

* Resolve merge conflicts

* Fix tests and linter

* Fix linter errors

* Linting issues

* Extra empty line

Co-authored-by: Bas <[email protected]>
reimda pushed a commit that referenced this pull request May 20, 2021
…t-go (#8937)

* new k8 client

* Make all tests pass

* Update licenses

* add timeout back

* Resolve merge conflicts

* Fix tests and linter

* Fix linter errors

* Linting issues

* Extra empty line

Co-authored-by: Bas <[email protected]>
(cherry picked from commit 79f5803)
@varunjain0606
Copy link
Contributor

@sspaink I am observing some issue related to the changes in this line and related logic.

fields["capacity_cpu_cores"] = convertQuantity(string(val.Format), 1)

I am getting a string(val.Format) value as either "DecimalSI" or "BinarySI" and hence getting error while parsing these values.
Logs from parser:

2021-08-03T03:37:20Z D! [inputs.kube_inventory] failed to parse quantity: quantities must match the regular expression '^([+-]?[0-9.]+)([eEinumkKMGTP][-+]?[0-9])$'
2021-08-03T03:37:20Z D! [inputs.kube_inventory] failed to parse quantity: quantities must match the regular expression '^([+-]?[0-9.]+)([eEinumkKMGTP][-+]?[0-9])$'
2021-08-03T03:37:20Z D! [inputs.kube_inventory] failed to parse quantity: quantities must match the regular expression '^([+-]?[0-9.]+)([eEinumkKMGTP][-+]?[0-9])$'
2021-08-03T03:37:20Z D! [inputs.kube_inventory] failed to parse quantity: quantities must match the regular expression '^([+-]?[0-9.]+)([eEinumkKMGTP][-+]?[0-9])$'

Once I convert string(val.Format) to val.String(), things seem to work fine. May I know why format is used as input to
convertQuantity() function? Is it supposed to be like that? If yes then what could be the issue.

Similar bug has been raised already.
#9535

@sspaink
Copy link
Contributor Author

sspaink commented Aug 4, 2021

@varunjain0606 Thank you for reporting this issue, looks like using format was a mistake. I noticed your pr #9581 to fix this, thank you for resolving it so quickly!

@varunjain0606
Copy link
Contributor

@sspaink No problem. Glad to help. Cheers!

@gracewehner
Copy link
Contributor

Hi @sspaink I found an issue related to the change in the watchPod() function in the file inputs/prometheus/kubernetes.go. I have created an issue #9600 and PR #9605 to address this.

arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
…t-go (influxdata#8937)

* new k8 client

* Make all tests pass

* Update licenses

* add timeout back

* Resolve merge conflicts

* Fix tests and linter

* Fix linter errors

* Linting issues

* Extra empty line

Co-authored-by: Bas <[email protected]>
(cherry picked from commit 79f5803)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants