fix static check failures #81940

carlory · 2019-08-26T12:32:59Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Errors from staticcheck:

plugin/pkg/admission/imagepolicy/admission.go:87:2: field retryBackoff is unused (U1000)
plugin/pkg/admission/imagepolicy/admission_test.go:1060:4: this value of err is never used (SA4006)
plugin/pkg/admission/imagepolicy/certs_test.go:22:5: var caKey is unused (U1000)
plugin/pkg/admission/imagepolicy/certs_test.go:70:5: var badCAKey is unused (U1000)

plugin/pkg/admission/podnodeselector/admission.go:235:2: this value of labelsMap is never used (SA4006)

hack/make-rules/helpers/go2make/testdata/dir-with-gofiles/bar.go:19:6: func bar is unused (U1000)
hack/make-rules/helpers/go2make/testdata/dir-with-gofiles/foo.go:19:6: func foo is unused (U1000)

pkg/apis/scheduling/v1alpha1/defaults.go:27:6: func addDefaultingFuncs is unused (U1000)

pkg/apis/scheduling/v1beta1/defaults.go:27:6: func addDefaultingFuncs is unused (U1000)

Which issue(s) this PR fixes:

Ref: #81657

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.:

NONE

k8s-ci-robot · 2019-08-26T12:33:08Z

Hi @carlory. 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.

carlory · 2019-08-26T13:17:23Z

/assign @liggitt

fejta-bot · 2019-08-26T13:28:37Z

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

carlory · 2019-09-04T07:05:34Z

@liggitt

It's ready for review. Can you give me an ok-to-test label on this pr?

mikedanese · 2019-09-06T01:30:33Z

plugin/pkg/admission/imagepolicy/certs_test.go

 -----END RSA PRIVATE KEY-----`)

 var clientCert = []byte(`-----BEGIN CERTIFICATE-----
-MIIC+jCCAeKgAwIBAgIJAMvo2rkGpEURMA0GCSqGSIb3DQEBCwUAMCExHzAdBgNV


Why is this showing a diff?

@mikedanese

the certs_test.go file is generated by the gencerts.sh script.

// This file was generated using openssl by the gencerts.sh script // and holds raw certificates for the imagepolicy webhook tests.

I changed the script and regenerated the certs_test.go file, so the file contents is changed.

//lint:file-ignore U1000 Ignore all unused code, it's generated

I'd recommend reverting the diffs in this file other than the lint comment

@liggitt

thanks for your review. it's reverted now.

carlory · 2019-09-26T06:46:43Z

@mikedanese

It's ready for re-review.

liggitt · 2019-09-26T15:00:50Z

/lgtm
/approve

alejandrox1

/ok-to-test
/priority important-longterm
/sig scheduling

pkg/apis/scheduling/v1alpha1/register.go

carlory · 2019-09-27T05:56:33Z

/test pull-kubernetes-bazel-test
/test pull-kubernetes-integration

carlory · 2019-09-27T07:15:32Z

/test pull-kubernetes-bazel-test

carlory · 2019-09-27T13:21:14Z

/test pull-kubernetes-bazel-test

liggitt · 2019-09-27T14:38:52Z

/retest
/lgtm
/approve

fejta-bot · 2019-09-27T16:53:04Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2019-09-27T19:19:38Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2019-09-27T21:47:06Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2019-09-28T00:14:05Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

carlory · 2019-09-28T01:50:02Z

/hold

liggitt · 2019-10-07T16:41:48Z

plugin/pkg/admission/imagepolicy/admission_test.go

@@ -1057,7 +1057,10 @@ func TestReturnedAnnotationAdd(t *testing.T) {
 			annotations := make(map[string]string)
 			attr = &fakeAttributes{attr, annotations}

-			err = wh.Validate(context.TODO(), attr, nil)
+			if err := wh.Validate(context.TODO(), attr, nil); err != nil {


err is expected to be non-nil for test: "Adds annotations for bad request",... need to indicate that in the testcase

thanks for your guide. I will do it later.

liggitt · 2019-10-07T16:41:55Z

/approve cancel

carlory · 2019-10-10T13:59:00Z

/hold cancel

carlory · 2019-10-10T14:01:25Z

/retest

liggitt · 2019-10-10T14:15:02Z

plugin/pkg/admission/imagepolicy/admission_test.go

+				if err == nil {
+					t.Errorf("%s: expected error making admission request: %v", tt.test, err)
+				}
+				return


don't return here, it skips the annotations equality check which is the point of the test

liggitt · 2019-10-10T14:15:29Z

plugin/pkg/admission/imagepolicy/admission_test.go

+				}
+				return
+			}
+			if err != nil {


Suggested change

if err != nil {

} else if err != nil {

liggitt · 2019-10-10T15:11:40Z

/lgtm
/approve

k8s-ci-robot · 2019-10-10T15:14:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: carlory, liggitt

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:

~~hack/OWNERS~~ [liggitt]
~~pkg/apis/OWNERS~~ [liggitt]
~~plugin/pkg/admission/OWNERS~~ [liggitt]

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

fix static check failures

k8s-ci-robot requested review from derekwaynecarr and mikedanese August 26, 2019 12:34

k8s-ci-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Aug 26, 2019

k8s-ci-robot assigned liggitt Aug 26, 2019

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 4, 2019

mikedanese reviewed Sep 6, 2019

View reviewed changes

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 26, 2019

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 26, 2019

alejandrox1 reviewed Sep 26, 2019

View reviewed changes

pkg/apis/scheduling/v1alpha1/register.go Show resolved Hide resolved

k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. labels Sep 26, 2019

k8s-ci-robot assigned alejandrox1 Sep 26, 2019

k8s-ci-robot added the sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. label Sep 26, 2019

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 27, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 27, 2019

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 28, 2019

liggitt reviewed Oct 7, 2019

View reviewed changes

k8s-ci-robot removed approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 7, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 10, 2019

liggitt reviewed Oct 10, 2019

View reviewed changes

fix static check failures

f6bb241

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 10, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 10, 2019

k8s-ci-robot merged commit 4f1c5b8 into kubernetes:master Oct 10, 2019

ohsewon pushed a commit to ohsewon/kubernetes that referenced this pull request Oct 16, 2019

Merge pull request kubernetes#81940 from carlory/fix-appserver

ed4e12a

fix static check failures

carlory deleted the fix-appserver branch April 2, 2020 10:03

carlory mentioned this pull request Jan 5, 2021

REQUEST: New membership for carlory kubernetes/org#2402

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix static check failures #81940

fix static check failures #81940

carlory commented Aug 26, 2019

k8s-ci-robot commented Aug 26, 2019

carlory commented Aug 26, 2019

fejta-bot commented Aug 26, 2019

carlory commented Sep 4, 2019

mikedanese Sep 6, 2019

carlory Sep 6, 2019

liggitt Sep 26, 2019

carlory Sep 26, 2019

carlory commented Sep 26, 2019

liggitt commented Sep 26, 2019

alejandrox1 left a comment

carlory commented Sep 27, 2019

carlory commented Sep 27, 2019

carlory commented Sep 27, 2019

liggitt commented Sep 27, 2019

fejta-bot commented Sep 27, 2019

fejta-bot commented Sep 27, 2019

fejta-bot commented Sep 27, 2019

fejta-bot commented Sep 28, 2019

carlory commented Sep 28, 2019

liggitt Oct 7, 2019

carlory Oct 10, 2019

liggitt commented Oct 7, 2019

carlory commented Oct 10, 2019

carlory commented Oct 10, 2019

liggitt Oct 10, 2019

carlory Oct 10, 2019

liggitt Oct 10, 2019

carlory Oct 10, 2019

liggitt commented Oct 10, 2019

k8s-ci-robot commented Oct 10, 2019

fix static check failures #81940

fix static check failures #81940

Conversation

carlory commented Aug 26, 2019

k8s-ci-robot commented Aug 26, 2019

carlory commented Aug 26, 2019

fejta-bot commented Aug 26, 2019

carlory commented Sep 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlory commented Sep 26, 2019

liggitt commented Sep 26, 2019

alejandrox1 left a comment

Choose a reason for hiding this comment

carlory commented Sep 27, 2019

carlory commented Sep 27, 2019

carlory commented Sep 27, 2019

liggitt commented Sep 27, 2019

fejta-bot commented Sep 27, 2019

fejta-bot commented Sep 27, 2019

fejta-bot commented Sep 27, 2019

fejta-bot commented Sep 28, 2019

carlory commented Sep 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented Oct 7, 2019

carlory commented Oct 10, 2019

carlory commented Oct 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented Oct 10, 2019

k8s-ci-robot commented Oct 10, 2019