tricorder-observability / Starship

Starship: next-generation Observability platform built with eBPF+WASM
GNU Affero General Public License v3.0
163 stars 23 forks source link

Feature: get apiaddress for kubernetes cluster. #74

Closed yaoyaoio closed 1 year ago

yaoyaoio commented 1 year ago

add: get apiaddress for kubernetes cluster.

I tested it on aliyun ack cluster.

CleanShot 2023-02-21 at 18 59 46@2x
nascentcore-eng commented 1 year ago

Take a look at the CI test failures, and see if you could fix them.

nascentcore-eng commented 1 year ago

Run tools/cleanup.sh to fix some of the style issues and bazel build files issue.

The below are the failures in the CI pipeline. image

yaoyaoio commented 1 year ago

ok. I fix these issues.

yaoyaoio commented 1 year ago

I have made changes according suggestions Please review and CI test.

nascentcore-eng commented 1 year ago

The go file lacks license header. You'll need to run tools/cleanup.sh to add the license header

nascentcore-eng commented 1 year ago

Starship's CI/CD setup on GitHub might prevent you from releasing a dev image, we'll fix them separately.

nascentcore-eng commented 1 year ago

I think my last request is to think about adding an automated test.

I could not come up a plan to implement a simple automated test, because there is no readily available testing fixture (like a fake kubernetes and kubectl local environment).

But I think you can write a brief doc on how to verify the correctness of this feature.

JaredTan95 commented 1 year ago

Starship's CI/CD setup on GitHub might prevent you from releasing a dev image, we'll fix them separately.

https://github.com/tricorder-observability/starship/pull/99 fixed docker build and push issue, you can rebase main branch to fix CI. @yaoyaoio

yaoyaoio commented 1 year ago

implement

Do you want me to implement a simple automated test to verify getting starship apiserver from kubernetes?

nascentcore-eng commented 1 year ago

Yes, if it's possible to do that.

If it's very complicated to do, then don't bother, manual testing is fine.

yaoyaoio commented 1 year ago

Sorry I should’t have used merge.

nascentcore-eng commented 1 year ago

Need to rebase on the main branch.