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

Code freeze on remote loader #4756

Closed
KnVerey opened this issue Aug 11, 2022 · 4 comments
Closed

Code freeze on remote loader #4756

KnVerey opened this issue Aug 11, 2022 · 4 comments
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/flake Categorizes issue or PR as related to a flaky test. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@KnVerey
Copy link
Contributor

KnVerey commented Aug 11, 2022

As described in Open letter to Kubernetes reviewers/approvers (and contributors!), the Kubernetes project is taking steps to improve the quality of our codebases for the long term. Notably:

We don’t want to ask contributors to jump through too many hoops to contribute, but given the current quality of Kubernetes, we’d like to change the bar for accepting a PR from “don’t make the code locally worse” to “also don’t build on shaky foundations”.

One area that this is particularly relevant for Kustomize is the code and tests that support the ability to load remote Kustomizations. We had two recent regressions showing the need for better coverage in this area: #4559 and #4455. #4615 attempted to fix this, but several issues remain.

  • Many of the new tests are skipped on CI because of Server lacks SSH keys #4620. They can still be run locally (which we do as part of the release process), but this is not a state we consider acceptable. We need to get all of the skips out of that test suite.
  • Many of the remote tests are flakey, some so badly that they are being skipped: Flaky integration tests #4640

Before we accept any changes to this area, we need all of these skips and flakes to be resolved so that we are building on a solid foundation. Going forward, we will require solid, stable testing for all new additions to this code. We acknowledge that this can be difficult given that many of the outstanding feature requests in this area require remote infrastructure we do not currently support. But it is not good for us or for our users if we cannot validate that the features they rely on will continue to work.

@KnVerey KnVerey added kind/flake Categorizes issue or PR as related to a flaky test. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Aug 11, 2022
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Aug 11, 2022
@annasong20
Copy link
Contributor

+1 It would definitely be nice to have more confidence in the CI pipeline and avoid all the "/retest"s 😄.

@natasha41575
Copy link
Contributor

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 12, 2022
annasong20 added a commit to annasong20/kustomize that referenced this issue Aug 26, 2022
annasong20 added a commit to annasong20/kustomize that referenced this issue Aug 29, 2022
Remove code that changes remotes-loading code path, as mandated by kubernetes-sigs#4756
@KnVerey
Copy link
Contributor Author

KnVerey commented Sep 23, 2022

@natasha41575 @annasong20 and I have agreed that thanks to @mightyguava great work, we can lift this freeze! 🎉
In its place, we will be freezing a much more specific piece of code that is involved in all Kustomization loading (remote and local) and was discovered to have poor error coverage as part of our efforts here. The new issue for that is #4807.

/close

@k8s-ci-robot
Copy link
Contributor

@KnVerey: Closing this issue.

In response to this:

@natasha41575 @annasong20 and I have agreed that thanks to @mightyguava great work, we can lift this freeze! 🎉
In its place, we will be freezing a much more specific piece of code that is involved in all Kustomization loading (remote and local) and was discovered to have poor error coverage as part of our efforts here. The new issue for that is #4807.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/flake Categorizes issue or PR as related to a flaky test. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

4 participants