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 running e2e tests as presubmits on GitHub #30927

Closed
wants to merge 9 commits into from
Closed

Allow running e2e tests as presubmits on GitHub #30927

wants to merge 9 commits into from

Conversation

uroy-personal
Copy link
Contributor

No description provided.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 1, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: hongkailiu / name: Hongkai Liu (a278493)
  • ✅ login: ameukam / name: Arnaud M. (b36e534)
  • ✅ login: rjsadow / name: Ricky Sadowski (9aa814e)
  • ✅ login: Priyankasaggu11929 / name: Priyanka Saggu (37409a8)
  • ✅ login: upodroid / name: Mahamed Ali (44d581f)
  • ✅ login: uroy-personal / name: Umesh Roy (ffc4ccb, 90b19c6, 175b482, 1fbd7a5)

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Oct 1, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @uroy-personal. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/config Issues or PRs related to code in /config area/jobs sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Oct 1, 2023
@uroy-personal uroy-personal changed the title Allow running e2e tests as presubmits on GitHub #5918 Allow running e2e tests as presubmits on GitHub Oct 1, 2023
@uroy-personal
Copy link
Contributor Author

For the issue : kubernetes/autoscaler#5918

@uroy-personal
Copy link
Contributor Author

/assign @jbartosik @kgolab @pbetkier

@uroy-personal
Copy link
Contributor Author

Good Morning @jbartosik , @kgolab , @pbetkier , @mwielgus & @bskiba.

I had to close the PR : #30893 as my fork got corrupted while rebasing the changes from parent repository.

@pbetkier , All the review comments from the previous PR has been incorporated here. I would like to request your review again on this PR.
Thanks again for your thorough assistance on this change.

@uroy-personal
Copy link
Contributor Author

I have signed the EasyCLA already. Hope the "EasyCLA — Missing CLA Authorization" check will be cleared soon

@pbetkier
Copy link
Contributor

pbetkier commented Oct 2, 2023

/ok-to-test

Looks fine to me, but let @kwiesmueller handle this as the new owner :)

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 2, 2023
@kwiesmueller
Copy link
Member

Tests fail with:

could not write config: 1 error occurred:
	* found duplicate name after normalizing: (DashboardTab) autoscalingvpafull

I think you have to rename the new presubmits.

@kwiesmueller
Copy link
Member

Or rather use a different dashboard name?

@uroy-personal
Copy link
Contributor Author

uroy-personal commented Oct 2, 2023

Good Morning @kwiesmueller ,
Yes this dashboard name has been used in the sig-autoscaling-config.yaml ( line no 139 ). Let me change the name. What name should we use?
autoscaling-vpa-full-presubmits?

@kwiesmueller
Copy link
Member

That name sounds good, yes.

@uroy-personal
Copy link
Contributor Author

/retest

@uroy-personal
Copy link
Contributor Author

/retest-required

@umesh-roy-sp
Copy link

Hi @kwiesmueller ,
I made the changes and trigger retest. The pull-test-infra-verify-lint test is failing with the message:

config/prow/cluster/build/build_kubernetes-external-secrets_deployment.yaml
39:11 warning comment not indented like content (comments-indentation)
ERROR: yamllint failed

I have not been able to identity the issue so far. When I validate the whole file using a yamllint, I get no error. I will look into the issue later today.

@uroy-personal
Copy link
Contributor Author

/retest-required

@uroy-personal
Copy link
Contributor Author

Good Morning @kwiesmueller ,
There was some extra space ( line no - 160 ) leading to yamllint validation error. I removed those spaces yesterday night and got the unit test cases passed.
I have also signed the EasyCLA yesterday. Could you please help with the next steps on this PR. Thanks!

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/testgrid and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 4, 2023
@uroy-personal
Copy link
Contributor Author

uroy-personal commented Oct 4, 2023

Hi @jbartosik , @kwiesmueller
I got the EasyCLA step passed. Please approve. Thanks a lot..

@kwiesmueller
Copy link
Member

/assign maciekpytel smg247

@uroy-personal
Copy link
Contributor Author

Good afternoon @MaciekPytel , @smg247 ,
Request your review on this PR please.

@laoj2
Copy link

laoj2 commented Oct 6, 2023

/lgtm

@k8s-ci-robot
Copy link
Contributor

@laoj2: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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.

@@ -540,6 +540,33 @@ dashboards:
results_url_template:
url: https://prow.ci.openshift.org/job-history/<gcs_prefix>
test_group_name: periodic-ci-openshift-multiarch-master-nightly-4.13-ocp-e2e-upgrade-aws-ovn-arm64
- base_options: width=10&exclude-filter-by-regex=Monitor%5Cscluster&exclude-filter-by-regex=%5Eoperator.Run%20template.*container%20test%24
Copy link

Choose a reason for hiding this comment

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

Can you move the config/testgrids/openshift/... changes to another PR? Those appear to be unrelated to the VPA presubmit tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @laoj2 ,
I did not add or make any changes to config/testgrids/openshift/. When I raised the PR initially I could only see my changes in a single file. Now I see 3 files got changed as part of this PR.

No idea why config/testgrids/openshift/ changes are pulled in into this PR..

@k8s-ci-robot k8s-ci-robot added area/prow Issues or PRs related to prow area/prow/bump Updates to the k8s prow cluster sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. labels Oct 6, 2023
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 6, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: uroy-personal
Once this PR has been reviewed and has the lgtm label, please assign zetaab for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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
Copy link
Contributor

k8s-ci-robot commented Oct 6, 2023

@uroy-personal: The following tests 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-test-infra-prow-image-build-test ffc4ccb link true /test pull-test-infra-prow-image-build-test
pull-test-infra-integration ffc4ccb link true /test pull-test-infra-integration

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.

@uroy-personal uroy-personal closed this by deleting the head repository Oct 10, 2023
@uroy-personal
Copy link
Contributor Author

Good morning @laoj2 ,
While rebasing with the origin, my repository got so many conflicts for some reasons. It was taking too much time to fix for this change. So I deleted my fork and recreated it. This pull request is closed.
I have raised a new one: #30994

Could you please review it.
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config Issues or PRs related to code in /config area/jobs area/prow/bump Updates to the k8s prow cluster area/prow Issues or PRs related to prow area/testgrid cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.