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

kustomize: migrate presubmits to community clusters #29733

Merged
merged 5 commits into from
Jun 17, 2023

Conversation

ShivamTyagi12345
Copy link
Member

This PR transitions the kustomize-presubmit-master job from the default cluster to eks-prow-build-cluster 👍

ref: #29722

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 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 Jun 8, 2023
@ShivamTyagi12345
Copy link
Member Author

How can I get the test eks-prow-build-cluster to pass?

@natasha41575
Copy link
Contributor

@ShivamTyagi12345 I don't know anything about the migration or prow infrastructure, maybe you can reach out to someone there to figure out how to get the test to pass?

@rjsadow
Copy link
Contributor

rjsadow commented Jun 8, 2023

xslack: https://kubernetes.slack.com/archives/C09QZ4DQB/p1686255333214949

The test is failing because spec.container.resources is blank. You can see the complaints about the failing job Here. This is because the GKE clusters don't require resource requests/limits to be set whereas the community owned clusters do.

I would recommend setting some default values (you can ref #29629). Then reach out to the Kustomize OWNERS to see if they have any recommendations. If they're not sure then you can merge with the default values and iterate to fine tune the requests and limits if the jobs start failing.

@rjsadow
Copy link
Contributor

rjsadow commented Jun 14, 2023

/retest

cpu: 4000m
limits:
memory: "2000Mi"
cpu: 500m
Copy link
Contributor

Choose a reason for hiding this comment

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

This will need to be 4000m to match the request

Copy link
Member Author

Choose a reason for hiding this comment

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

I am doing this change. Let's look into the results

@ShivamTyagi12345
Copy link
Member Author

@rjsadow I see these values lead to the similar error

@rjsadow
Copy link
Contributor

rjsadow commented Jun 15, 2023

/retest

cpu: 4000m
limits:
memory: "2000Mi"
cpu: 4000m
Copy link
Contributor

@rjsadow rjsadow Jun 15, 2023

Choose a reason for hiding this comment

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

The linter is erroring on the trailing white space after the 4000m.

cpu: 4000m

config/jobs/kubernetes-sigs/kustomize/kustomize-presubmit-master.yaml
  23:23     error    trailing spaces  (trailing-spaces) 

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I have remove d the trailing space, How are you seeing that this was the error?

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there is still some trailing whitespace after the CPU. You can look at the log that the k8s-ci-robot provides or from the job itself. Here's the link to the job logs https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/test-infra/29733/pull-test-infra-verify-lint/1669558900354977792. In there if you just search for "error" you'll find the offending line(s).

@ameukam
Copy link
Member

ameukam commented Jun 17, 2023

/retitle kustomize: migrate presubmits to community clusters

@k8s-ci-robot k8s-ci-robot changed the title Edit the community cluster to kustomize kustomize: migrate presubmits to community clusters Jun 17, 2023
@ameukam
Copy link
Member

ameukam commented Jun 17, 2023

/approve
/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ameukam, ShivamTyagi12345

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 Jun 17, 2023
@k8s-ci-robot k8s-ci-robot merged commit e779457 into kubernetes:master Jun 17, 2023
@k8s-ci-robot
Copy link
Contributor

@ShivamTyagi12345: Updated the job-config configmap in namespace default at cluster test-infra-trusted using the following files:

  • key kustomize-presubmit-master.yaml using file config/jobs/kubernetes-sigs/kustomize/kustomize-presubmit-master.yaml

In response to this:

This PR transitions the kustomize-presubmit-master job from the default cluster to eks-prow-build-cluster 👍

ref: #29722

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
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/config Issues or PRs related to code in /config area/jobs cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. 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.

5 participants