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

Migrate away from google.com gcp project k8s-testimages #107

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cpanato
Copy link
Member

@cpanato cpanato commented Jun 7, 2023

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

  • Migrate away from google.com gcp project k8s-testimages
  • format indentation

Related to kubernetes/k8s.io#1523

/assign @ameukam @thockin @bowei

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Jun 7, 2023
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 7, 2023
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 7, 2023
@cpanato
Copy link
Member Author

cpanato commented Jun 7, 2023

/retest

Copy link

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Why test-infra and not a subproject-specific GCP project?

@cpanato
Copy link
Member Author

cpanato commented Jun 7, 2023

Why test-infra and not a subproject-specific GCP project?

good question to @ameukam

@ameukam
Copy link
Member

ameukam commented Jun 7, 2023

Why test-infra and not a subproject-specific GCP project?

k8s-staging-test-infra is currently is community-owned and used specifically to host the images maintained by SIG Testing. I don't correctly remember the specifics about why this name (early days of the migration to k8s-infra for those images) but my guess is test-infra "helps" to easily identify where the Dockerfiles of those images are located.

@thockin
Copy link

thockin commented Jun 7, 2023

test-infra doesn't seem right for testing ingress-controllers, though - we do already have k8s-staging-ingress-controller-conformance.

From as directional POV, what do we want as a project?

  1. humans don't touch artifacts
  2. bots and automation exclusively manage them

As such, it seems like building it in the same project that has the staging GCR is the right thing to do...

@thockin
Copy link

thockin commented Jul 7, 2023

What do we do with this? Approve or try to adjust?

@thockin
Copy link

thockin commented Oct 11, 2023

ping @robscott @bowei

@robscott
Copy link
Member

These images had been used for Gateway API, but we actually just completed a migration away from them today (kubernetes-sigs/gateway-api#2468). For the past several years, the only updates this repo was getting was just to update conformance images for Gateway API, so we decided to pull those into the main GW API repo and by extension k8s-staging-gateway-api in gcr.

I think this specific project is effectively archived at this point, and I don't know of anyone regularly running these tests. I'm not sure it's worth the effort to make changes here. Instead, I do know that the conformance images used by Gateway API will likely continue to get significant usage and updates, so if there are any similar improvements we could make there, definitely let us know.

@ameukam
Copy link
Member

ameukam commented Oct 11, 2023

Instead, I do know that the conformance images used by Gateway API will likely continue to get significant usage and updates, so if there are any similar improvements we could make there, definitely let us know.

@robscott we can close this PR and request to archive this repo is done by sig-network ?

@robscott
Copy link
Member

request to archive this repo is done by sig-network

I'm not 100% sure what to do here. We know that the Ingress API is essentially frozen and not going to change, which would suggest that these conformance tests would also not change. On the other hand, the Ingress API is not going away and archiving this repo may give the wrong message on that front. Interested in what others think here as well.

/cc @shaneutt @youngnick

@cpanato
Copy link
Member Author

cpanato commented Oct 12, 2023

I completely lost track of this, sorry

I think this PR is only to change the deprecated image; if other things need to be done to push this repo to an up-to-date state, we can do it in a follow-up. I volunteer myself to do all the required work.

@robscott @ameukam @thockin

Copy link

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Thanks!

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 12, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, thockin

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2023
@cpanato
Copy link
Member Author

cpanato commented Oct 13, 2023

@thockin we will need to override the failing checks to get this merged.

also what is the best slack channel to talk with you about next steps for this repo?

@thockin
Copy link

thockin commented Oct 14, 2023

sig-network is the right channel, but if you need specific people you probably need to name them :)

@thockin
Copy link

thockin commented Oct 14, 2023

/retest

I don't know much about these checks - @robscott ?

@k8s-ci-robot
Copy link
Contributor

@cpanato: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-ingress-controller-conformance-gherkin 9ab53be link true /test pull-ingress-controller-conformance-gherkin

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@youngnick
Copy link

Yeah, it makes sense to me to keep this around rather than going full archive.

@cpanato
Copy link
Member Author

cpanato commented Jan 20, 2024

this is something we can skip to get this PR merged?

cc @thockin @youngnick @robscott

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all 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:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this 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 Apr 19, 2024
@thockin thockin removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants