search

kubernetes-sigs / kubetest2

Kubetest2 is the framework for launching and running end-to-end tests on Kubernetes.
Apache License 2.0
326 stars 105 forks source link

Some updates to kubetest2 gke deployer #172

Closed chizhg closed 1 year ago

chizhg commented 2 years ago
  1. Change more fields to be public to use it as a library
  2. For multi-project multi-cluster topology, allow to not specify the project index in the cluster names, in which case the cluster index will be the same as the project index
  3. It's not useful to print the number of firewall rules deleted, so dropping the calculation and return logic
chizhg commented 2 years ago

/retest

chizhg commented 2 years ago

/assign @BenTheElder @MushuEE Could you help review this PR? Thanks!

BenTheElder commented 2 years ago

Change more fields to be public to use it as a library

Are these fields reasonable to export ...? A cursory glance shows some pretty internal stuff (like the Cluster index struct)

I don't think this package was intended to be consumed as a library, and we're extremely thin on maintainer time (... I am the sole maintainer now, and I'm maintaining a lot of other things).

It's not useful to print the number of firewall rules deleted, so dropping the calculation and return logic

debugging resource leaks ...?

chizhg commented 2 years ago

It's already used as a library - we are using it to implement an internal tool for Anthos, Amit reviewed all the previous PRs and this is changing more fields to be public.

We are also printing the errors, so printing the number of deleted firewall rules feels a bit redundant, but I can add it back if you think it's more desirable.

And sorry for bothering..., I'm now mostly wrapping up stuff and cleaning up some tech debt, so this should be my last change to this repo...

k8s-ci-robot commented 2 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chizhg To complete the pull request process, please ask for approval from bentheelder after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/kubernetes-sigs/kubetest2/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
BenTheElder commented 2 years ago

It's already used as a library - we are using it to implement an internal tool for Anthos, Amit reviewed all the previous PRs and this is changing more fields to be public.

I see that a few functions were made public previously.

I probably would have disagreed with this change -- deployer already has a standard public API, but also those changes are not the same as exposing types like the internal cluster index struct.

We are also printing the errors, so printing the number of deleted firewall rules feels a bit redundant, but I can add it back if you think it's more desirable.

I think it's useful to know the number. I don't think it's clearly not useful to know it.

[Bug fix] Boskos release should set the resource state to dirty

This seems like it should really be a separate PR.

For multi-project multi-cluster topology, allow to not specify the project index in the cluster names, in which case the cluster index will be the same as the project index

Have not had time to review this aspect, sorry, currently working on KETTLE outage .. this repo is low-prio bugfix focused right now until we can re-staff.

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-ci-robot commented 1 year ago

@chizhg: PR needs rebase.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot commented 1 year ago

@k8s-triage-robot: Closed this PR.

In response to [this](https://github.com/kubernetes-sigs/kubetest2/pull/172#issuecomment-1320721415): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages PRs according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the PR is closed > >You can: >- Reopen this PR with `/reopen` >- Mark this PR as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.