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

Allow pod-utils / clonerefs to use default remote branch #20667

Open
spiffxp opened this issue Jan 29, 2021 · 22 comments
Open

Allow pod-utils / clonerefs to use default remote branch #20667

spiffxp opened this issue Jan 29, 2021 · 22 comments
Labels
area/jobs area/prow/pod-utilities Issues or PRs related to prow's pod-utilities component kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/testing Categorizes an issue or PR as relevant to SIG Testing.
Milestone

Comments

@spiffxp
Copy link
Member

spiffxp commented Jan 29, 2021

My read of current pod-utils code is that it assumes base_ref to always be present / non-empty (ref: #20665 (comment))

It looks like there are 435 jobs that explicitly reference master, when what they likely mean is "just use whatever the default remote branch is"

$ ag --no-filename "base_ref:.*master" config/jobs | grep base_ref | wc -l
     435

Ideally base_ref would be auto-populated with the remote default branch if omitted. Maybe this already happens? This would make repo default branch renames less toilsome.

Opening this to track verifying or fixing, so not yet a bug or feature.

/area prow/pod-utilities

/assign @alvaroaleman @cjwagner
in case you can answer more quickly than I can, feel free to /unassign if you can't handle
/assign @spiffxp
to verify what happens today

@k8s-ci-robot k8s-ci-robot added the area/prow/pod-utilities Issues or PRs related to prow's pod-utilities component label Jan 29, 2021
@spiffxp
Copy link
Member Author

spiffxp commented Jan 29, 2021

/sig testing

@k8s-ci-robot k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Jan 29, 2021
@alvaroaleman
Copy link
Member

alvaroaleman commented Jan 29, 2021

My read of current pod-utils code is that it assumes base_ref to always be present / non-empty (ref: #20665 (comment))

Yeah, that is correct (for periodics, for presubmits/postubmits we get the info from github)

Ideally base_ref would be auto-populated with the remote default branch if omitted. Maybe this already happens? This would make repo default branch renames less toilsome.

I am pretty sure this does not happen. Adding this would be pretty easy from a complexity POV, as the default_branch is part of the repo. It will drive up token usage a bit though (and I think it won't cache that well, as the repo contains a lot of info that will regularly change, like pushed_at: timestamp, size, the number of stars, forks etc)

@spiffxp
Copy link
Member Author

spiffxp commented Jan 29, 2021

If we're concerned about token usage, we could try doing this directly in clone by using the contents of .git/refs/remotes/origin/HEAD if base_ref is empty. I dunno if that makes the API feel too weird though (technically it does omitempty as-is)

@spiffxp
Copy link
Member Author

spiffxp commented Jan 29, 2021

/area jobs
since this would be a job config change too

@spiffxp
Copy link
Member Author

spiffxp commented Jan 29, 2021

/wg naming
since this is in support of kubernetes/org#2222

@spiffxp
Copy link
Member Author

spiffxp commented Mar 8, 2021

Relevant discussion thread in #prow https://kubernetes.slack.com/archives/CDECRSC5U/p1615236463055600

@spiffxp
Copy link
Member Author

spiffxp commented Mar 9, 2021

Discussed at sig-testing meeting today:

  • Goal is to add support for this to extra_refs and not the spec for triggering presubmits/postsubmits
    • so, the extra_refs used by periodics
    • and also, any extra_refs used by presubmits/postsubmits that clone multiple repos
  • We might be able to use HEAD as part of the extra_refs spec today
  • If not, I'll try prototyping something, if it gets too complex, I'll pull back to a proposal for review

@stevekuznetsov
Copy link
Contributor

Since the HEAD symbolic ref is not a default git thing and instead a real symbolic ref that e.g. GitHub, Gerrit, GitLab provide, it should be possible to just use extra_refs[*].base_ref: HEAD.

@spiffxp
Copy link
Member Author

spiffxp commented Mar 9, 2021

FWIW, I feel like remotes having a HEAD is a default git thing, but maybe I'm wrong. I'll give the base_ref: HEAD thing a shot this week

refs:

@BenTheElder
Copy link
Member

not quite workable, because we try to check it out to a branch with that name, and it's not a valid branch name #21452 (comment)

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 17, 2021
@BenTheElder
Copy link
Member

I think we could add something simple to replace HEAD with a valid branch name and should still consider this.

@spiffxp
Copy link
Member Author

spiffxp commented Jul 13, 2021

/remove-lifecycle stale
/milestone v1.23

@spiffxp
Copy link
Member Author

spiffxp commented Nov 24, 2021

$ ag --no-filename "base_ref:.*master" config/jobs | grep base_ref | wc -l
    1141

The number of jobs affected has grown considerably from the 435 that existed at the beginning of this year

@spiffxp
Copy link
Member Author

spiffxp commented Nov 24, 2021

/milestone v1.24

@k8s-ci-robot k8s-ci-robot modified the milestones: v1.23, v1.24 Nov 24, 2021
@spiffxp
Copy link
Member Author

spiffxp commented Nov 24, 2021

/priority important-longterm

@k8s-ci-robot k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Nov 24, 2021
@k8s-triage-robot
Copy link

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:

  • 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 issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 22, 2022
@k8s-triage-robot
Copy link

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:

  • 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 issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 24, 2022
@BenTheElder BenTheElder removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Apr 19, 2022
@BenTheElder
Copy link
Member

@cjwagner @stevekuznetsov @chaodaiG WDYT #20667 (comment) ?

I think we could do this pretty easily by just detecting HEAD => pick a different value for the branch name but otherwise use HEAD in most places untouched. Last time I tried this the branch name seemed to be the only stumbling block. I'm not sure what we should call it.

@BenTheElder BenTheElder modified the milestones: v1.24, someday Apr 19, 2022
@BenTheElder BenTheElder removed the wg/naming Categorizes an issue or PR as relevant to WG Naming. label Apr 19, 2022
@k8s-triage-robot
Copy link

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:

  • 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 issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 18, 2022
@BenTheElder BenTheElder added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 27, 2022
@BenTheElder
Copy link
Member

This would now be at https://sigs.k8s.io/prow

I think so far we've just wound up updating the configs as necessary. I do think this would be nice to have, but there clearly haven't been the resources and this is no longer the place to track prow features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/jobs area/prow/pod-utilities Issues or PRs related to prow's pod-utilities component kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. sig/testing Categorizes an issue or PR as relevant to SIG Testing.
Projects
None yet
Development

No branches or pull requests

8 participants