kubernetes / test-infra

Test infrastructure for the Kubernetes project.
Apache License 2.0
3.84k stars 2.66k forks source link

[peribolos] Adding sub team to a parent team fails during dry-run #26331

Closed dprotaso closed 4 months ago

dprotaso commented 2 years ago

@dprotaso it is trying to list team members of the Serving WG Leads which is newly added. Since you are running this without confirm it doesn't actually create the team, and can't list members. This doesn't have to do with my changes to slugs, and I am not convinced that this scenario would have ever worked properly. afaik the reason it is listing team members is due to the team having a parent, so this may be a less utilized scenario that hasn't been accounted for.

Note: this isn't really a blocker for you, all you need to do is actually create the team, and then everything will be fine.

Originally posted by @smg247 in https://github.com/kubernetes/test-infra/issues/26234#issuecomment-1127556164

dprotaso commented 2 years ago

/sig k8s-infra

dprotaso commented 2 years ago

Logs

{"component":"peribolos","file":"k8s.io/test-infra/prow/cmd/peribolos/main.go:1297","func":"main.configureTeamMembers.func1","level":"info","msg":"Set dprotaso as a member of api-core-wg-leads(API Core WG Leads)","severity":"info","time":"2022-05-13T20:35:00Z"}
[415](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_community/1067/pull-knative-peribolos/1525211406578749440#1:build-log.txt%3A415)

{"client":"github","component":"peribolos","file":"k8s.io/test-infra/prow/github/client.go:910","func":"k8s.io/test-infra/prow/github.(*client).log","level":"info","msg":"EditTeam({0 Serving WG Leads serving-wg-leads  closed \u003cnil\u003e 0xc000181880 })","severity":"info","time":"2022-05-13T20:35:00Z"}
[416](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_community/1067/pull-knative-peribolos/1525211406578749440#1:build-log.txt%3A416)

{"client":"github","component":"peribolos","file":"k8s.io/test-infra/prow/github/client.go:910","func":"k8s.io/test-infra/prow/github.(*client).log","level":"info","msg":"ListTeamMembersBySlug(knative, serving-wg-leads, member)","severity":"info","time":"2022-05-13T20:35:00Z"}
[417](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_community/1067/pull-knative-peribolos/1525211406578749440#1:build-log.txt%3A417)

{"component":"peribolos","file":"k8s.io/test-infra/prow/cmd/peribolos/main.go:209","func":"main.main","level":"fatal","msg":"Configuration failed: failed to configure knative teams: failed to update Serving Writers child teams: failed to update Serving WG Leads members: failed to list serving-wg-leads(Serving WG Leads) members: return code not 2XX: 404 Not Found","severity":"fatal","time":"2022-05-13T20:35:10Z"}
dprotaso commented 2 years ago

cc @cjwagner @fejta

fejta commented 2 years ago

Are you offering to fix this edge case?

dprotaso commented 2 years ago

I'd probably make things worse ;)

fejta commented 2 years ago

I'm mostly inclined to just resolve this without fixing anything.

You have declared a non-existent team for weeks. Why is this an important scenario? Just create the group (which peribolos will do for you if you use the --confirm flag, right?).

dprotaso commented 2 years ago

My mental model for using this tool is

  1. Run dry-run to ensure config etc is valid
  2. Apply the changes when 1 succeeds

We do 1 in PR presubmits and 2 in post submits.

My impression is this tool is meant to declaratively manage GitHub orgs - using the GitHub UI as a work around feels at odds with that.

dprotaso commented 2 years ago

Likewise using the confirm flag prior to the dry run seems like a workaround

fejta commented 2 years ago

A dry run when you're declaring and using a team that doesn't exist isn't going to work though.

You could break it up into multiple PRs: 1) a PR where you declare the new team but don't use it anywhere 2) After merging (1) then you start using the team as expected.

This should work properly with dry runs

fejta commented 2 years ago

Alternatively you can disable the functionality that is not of interest: https://github.com/kubernetes/test-infra/blob/fb4c8c9bd8603ec9306bf3a59119274feb961f7f/prow/cmd/peribolos/main.go#L83-L102

dprotaso commented 2 years ago

I guess multiple PRs is a decent enough workaround. I'll leave this issue open in case someone else is looking to pick up the work.

dprotaso commented 2 years ago

Splitting things up into a separate PRs didn't work. It seems like --confirm=false is broken? I created a new issue: https://github.com/kubernetes/test-infra/issues/26542

k8s-triage-robot commented 2 years 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

dprotaso commented 2 years ago

/lifecycle frozen

BenTheElder commented 4 months ago

/sig contribex

This tool moved to https://github.com/kubernetes-sigs/prow

If you're still interested in this, please re-file with the new repo, we're closing out old inactive issues here.

k8s-ci-robot commented 4 months ago

@BenTheElder: The label(s) sig/contribex cannot be applied, because the repository doesn't have them.

In response to [this](https://github.com/kubernetes/test-infra/issues/26331#issuecomment-2253290333): >/sig contribex > >This tool moved to https://github.com/kubernetes-sigs/prow > >If you're still interested in this, please re-file with the new repo, we're closing out old inactive issues here. 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-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.