Skip to content

Temporarily ignore some verify failures in go1.18 and other broken things#108901

Merged
k8s-ci-robot merged 5 commits intokubernetes:masterfrom
dims:temporarily-ignore-some-verify-failures-in-go-1.18
Mar 23, 2022
Merged

Temporarily ignore some verify failures in go1.18 and other broken things#108901
k8s-ci-robot merged 5 commits intokubernetes:masterfrom
dims:temporarily-ignore-some-verify-failures-in-go-1.18

Conversation

@dims
Copy link
Copy Markdown
Member

@dims dims commented Mar 23, 2022

Some of the verify scripts are failing after the go 1.18 based images made it to prow.

Signed-off-by: Davanum Srinivas [email protected]

What type of PR is this?

/kind failing-test

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Mar 23, 2022
@dims dims changed the title Temporarily ignore some verify failures in go1.18" [WIP] Temporarily ignore some verify failures in go1.18" Mar 23, 2022
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 23, 2022
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 23, 2022
@jiahuif jiahuif mentioned this pull request Mar 23, 2022
Merged
@dims
Copy link
Copy Markdown
Member Author

dims commented Mar 23, 2022

/retest

@kolyshkin kolyshkin mentioned this pull request Mar 23, 2022
Merged
@kolyshkin
Copy link
Copy Markdown
Contributor

kolyshkin commented Mar 23, 2022

@dims most probably you need to update to golangci-lint 1.45.0, which supports go 1.18.

Actually, here's a PR to try this: #108902

@kolyshkin kolyshkin mentioned this pull request Mar 23, 2022
Merged
@dims
Copy link
Copy Markdown
Member Author

dims commented Mar 23, 2022

@kolyshkin looks like we got hit by something that just got fixed but not released yet - golangci/golangci-lint#2649 (comment)

@dims
Copy link
Copy Markdown
Member Author

dims commented Mar 23, 2022

am testing some other changes to avoid ignoring things here - #108900

@kolyshkin
Copy link
Copy Markdown
Contributor

kolyshkin commented Mar 23, 2022

@kolyshkin looks like we got hit by something that just got fixed but not released yet - golangci/golangci-lint#2649 (comment)

It seems that golangci-lint 1.45.0 works fine (see #108902), but there are a bunch of other go 1.18 related changes. I hope I have fixed some more of those in that PR (#108902).

@adisky adisky mentioned this pull request Mar 23, 2022
Merged
@dims
Copy link
Copy Markdown
Member Author

dims commented Mar 23, 2022

/retest

@dims dims changed the title [WIP] Temporarily ignore some verify failures in go1.18" Temporarily ignore some verify failures in go1.18" Mar 23, 2022
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 23, 2022
@dims dims changed the title Temporarily ignore some verify failures in go1.18" Temporarily ignore some verify failures in go1.18 Mar 23, 2022
@ameukam
Copy link
Copy Markdown
Member

ameukam commented Mar 23, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 23, 2022
@cpanato
Copy link
Copy Markdown
Member

cpanato commented Mar 23, 2022

/triage accepted
/priority critical-urgent

@k8s-ci-robot k8s-ci-robot added sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 23, 2022
@dims
Copy link
Copy Markdown
Member Author

dims commented Mar 23, 2022

/priority critical-urgent
/kind failing-test

@dims dims force-pushed the temporarily-ignore-some-verify-failures-in-go-1.18 branch from b9d2449 to 0a5f911 Compare March 23, 2022 12:03
@dims
Copy link
Copy Markdown
Member Author

dims commented Mar 23, 2022

/assign @aojea @MadhavJivrajani

@dims
Copy link
Copy Markdown
Member Author

dims commented Mar 23, 2022

@kolyshkin looks like we got hit by something that just got fixed but not released yet - golangci/golangci-lint#2649 (comment)

It seems that golangci-lint 1.45.0 works fine (see #108902), but there are a bunch of other go 1.18 related changes. I hope I have fixed some more of those in that PR (#108902).

Ack. Let me see how far i get with the things here. I have the golangci-lint commented out in this PR temporarily, we can re-enable it in your PR @kolyshkin thanks for digging in!

@MadhavJivrajani
Copy link
Copy Markdown
Contributor

MadhavJivrajani commented Mar 23, 2022

Thanks dims!

@dims dims changed the title Temporarily ignore some verify failures in go1.18 Temporarily ignore some verify failures in go1.18 and other broken things Mar 23, 2022
@ameukam
Copy link
Copy Markdown
Member

ameukam commented Mar 23, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 23, 2022
cpanato
cpanato approved these changes Mar 23, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

/lgtm

endocrimes
endocrimes approved these changes Mar 23, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@endocrimes endocrimes left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Mar 23, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, dims, endocrimes

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

The pull request process is described here

Details 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 merged commit 1580b69 into kubernetes:master Mar 23, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.24 milestone Mar 23, 2022
@mdbooth mdbooth mentioned this pull request Mar 23, 2022
Merged
@MadhavJivrajani MadhavJivrajani mentioned this pull request Mar 23, 2022
Closed
@liggitt
Copy link
Copy Markdown
Member

liggitt commented Mar 23, 2022

thanks for the quick fix here. are there tracker issues for each of the removed checks in the 1.24 milestone that owning sigs are aware of?

@MadhavJivrajani MadhavJivrajani mentioned this pull request Mar 23, 2022
Closed
@pohly pohly mentioned this pull request Mar 24, 2022
Merged
@liggitt liggitt mentioned this pull request Nov 17, 2022
Closed
4 tasks
@liggitt liggitt mentioned this pull request Dec 16, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpanato cpanato cpanato approved these changes

@mikedanese mikedanese Awaiting requested review from mikedanese

@SataQiu SataQiu Awaiting requested review from SataQiu

+1 more reviewer

@endocrimes endocrimes endocrimes approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@endocrimes endocrimes

@ameukam ameukam

@cpanato cpanato

@aojea aojea

@MadhavJivrajani MadhavJivrajani

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note-none Denotes a PR that doesn't merit a release note. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

None yet

Milestone

v1.24

Development

Successfully merging this pull request may close these issues.

9 participants

@dims @kolyshkin @ameukam @cpanato @MadhavJivrajani @k8s-ci-robot @liggitt @endocrimes @aojea