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

Collect Kubernetes distribution platform and version #1651

Merged
merged 24 commits into from
Mar 13, 2024

Conversation

bjee19
Copy link
Contributor

@bjee19 bjee19 commented Mar 7, 2024

Problem: We want to collect the Kubernetes distribution platform and version to ensure NGF works well with the most popular platforms and versions.

Solution: Added collection of Kubernetes distribution platform and version.

Testing: Added unit tests. Conducted E2E testing on each of the specified platforms in #1308 to ensure the platform is correctly collected.

Closes #1308

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.


@github-actions github-actions bot added the enhancement New feature or request label Mar 7, 2024
@bjee19
Copy link
Contributor Author

bjee19 commented Mar 7, 2024

Below is manually gathered values of a node's spec.providerID for different platforms. All nodes of a platform had the same value in the providerID.

aks:
providerID: azure:///subscriptions/…

gke:
providerID: gce:///us-central1-c/

openshift:
providerID: missing

eks:
providerID: aws:///us-west-2c/***

rancher:
providerID: k3s://local-node

k3s:
providerID: k3s://ip-172-16-0-210

kind:
providerID: kind://docker/kind/kind-control-plane

@bjee19
Copy link
Contributor Author

bjee19 commented Mar 7, 2024

Below is manually gathered values of a node’s Status.NodeInfo.KubletVersion in addition to the server gitVersion response when running kubectl version --v=8. All nodes of a platform had the same value in the Status.NodeInfo.KubletVersion.

aks:
kubletversion: “v1.28.5”
server gitVersion: “v1.28.5”

gke:
kubletversion: "v1.27.8-gke.1067004"
server gitVersion: "v1.27.8-gke.1067004"

openshift:
kubletversion: “v1.24.6+5157800”
server gitVersion: "v1.24.6+5157800"

eks:
kubeletversion: "v1.29.0-eks-a5ec690"
GitVersion:"v1.29.1-eks-508b6b3"

rancher:
kubletversion: “v1.27.6+k3s1”
server gitVersion: "v1.27.6+k3s1"

k3s:
kubletversion: “v1.28.6+k3s2”
server gitVersion: “v1.28.6+k3s2”

kind:
kubletversion: "v1.29.2"
server gitVersion: "v1.29.2”

They are the same except for eks.

internal/mode/static/usage/job_worker.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Show resolved Hide resolved
pkg/telemetry/platform.go Outdated Show resolved Hide resolved
pkg/telemetry/platform.go Outdated Show resolved Hide resolved
pkg/telemetry/platform.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
pkg/telemetry/platform.go Outdated Show resolved Hide resolved
@bjee19 bjee19 marked this pull request as ready for review March 11, 2024 15:10
@bjee19 bjee19 requested a review from a team as a code owner March 11, 2024 15:10
@bjee19 bjee19 requested a review from pleshakov March 11, 2024 15:10
@pleshakov
Copy link
Contributor

@bjee19 I recommend resolving the conflicts, so that we don't need to do another review after resolving

@bjee19 bjee19 force-pushed the enh/telemetry-collect-platform-version branch from a876b83 to 4411df2 Compare March 11, 2024 18:38
Copy link

codecov bot commented Mar 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.90%. Comparing base (ad0242b) to head (c970b6b).
Report is 5 commits behind head on main.

❗ Current head c970b6b differs from pull request most recent head a929f5a. Consider uploading reports for the commit a929f5a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1651      +/-   ##
==========================================
+ Coverage   85.83%   85.90%   +0.06%     
==========================================
  Files          86       87       +1     
  Lines        5459     5512      +53     
  Branches       52       52              
==========================================
+ Hits         4686     4735      +49     
- Misses        722      724       +2     
- Partials       51       53       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/platform.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector_test.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/platform.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Show resolved Hide resolved
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 13, 2024
@bjee19 bjee19 force-pushed the enh/telemetry-collect-platform-version branch from b7758e4 to 57efd63 Compare March 13, 2024 01:51
@bjee19 bjee19 requested a review from pleshakov March 13, 2024 16:03
@bjee19 bjee19 removed the documentation Improvements or additions to documentation label Mar 13, 2024
@bjee19
Copy link
Contributor Author

bjee19 commented Mar 13, 2024

E2E testing results by testing on each platform:

aks: "K8sVersion":"1.25.6","K8sPlatform":"aks”

gke: "K8sVersion":"1.27.8","K8sPlatform":"gke”

openshift: "K8sVersion":"1.29.2","K8sPlatform":"openshift” — NGF fails to deploy on Openshift, so to test this I created a Kind cluster then added the openshift identifier label to the kind node’s Labels. This should suffice testing for openshift. 

eks: "K8sVersion":"1.29.0","K8sPlatform":"eks”

rancher: "K8sVersion":"1.27.6","K8sPlatform":"rancher”

k3s: "K8sVersion":"1.28.7","K8sPlatform":"k3s”

Kind: "K8sVersion":"1.29.2","K8sPlatform":"kind"

internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Show resolved Hide resolved
@bjee19 bjee19 requested review from pleshakov and kate-osborn March 13, 2024 17:01
Copy link
Contributor

@kate-osborn kate-osborn left a comment

Choose a reason for hiding this comment

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

Looks good! Just a few small suggestions

internal/mode/static/telemetry/collector.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Show resolved Hide resolved
internal/mode/static/telemetry/collector.go Show resolved Hide resolved
internal/mode/static/telemetry/platform.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/platform.go Outdated Show resolved Hide resolved
internal/mode/static/telemetry/platform.go Outdated Show resolved Hide resolved
@bjee19 bjee19 requested a review from kate-osborn March 13, 2024 19:15
Copy link
Contributor

@kate-osborn kate-osborn left a comment

Choose a reason for hiding this comment

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

🚀

@bjee19 bjee19 force-pushed the enh/telemetry-collect-platform-version branch from a929f5a to 3550249 Compare March 13, 2024 21:48
@bjee19 bjee19 merged commit 770ff04 into nginxinc:main Mar 13, 2024
38 checks passed
amimimor pushed a commit to amimimor/nginx-gateway-fabric that referenced this pull request Apr 3, 2024
Problem: We want to collect the Kubernetes distribution platform and version to ensure NGF works well with the most popular platforms and versions.

Solution: Added collection of Kubernetes distribution platform and version.
@bjee19 bjee19 deleted the enh/telemetry-collect-platform-version branch May 7, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Collect Deployed Platform and Kubernetes Version (NGF)
6 participants