Skip to content

Conversation

@jpayne3506
Copy link
Contributor

@jpayne3506 jpayne3506 commented Jul 28, 2023

Reason for Change:

Did not match the design of other go tests, slowing down development.

Issue Fixed:

Requirements:

Notes:

@jpayne3506 jpayne3506 added the cni Related to CNI. label Jul 28, 2023
@jpayne3506 jpayne3506 self-assigned this Jul 28, 2023
@jpayne3506 jpayne3506 requested a review from a team as a code owner July 28, 2023 01:46
@jpayne3506 jpayne3506 force-pushed the jpayne3506/gowinrefactor branch from 7e6699d to bcd100a Compare July 28, 2023 17:08
paulyufan2
paulyufan2 previously approved these changes Jul 28, 2023
Copy link
Contributor

@paulyufan2 paulyufan2 left a comment

Choose a reason for hiding this comment

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

lgtm

restConfig := k8sutils.MustGetRestConfig(t)

t.Log("Create Clientset")
clientset, _ := k8sutils.MustGetClientset()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we ignoring the error ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question! Going to change this.

}
if len(pods.Items) <= 1 {
require.NoError(t, errors.New("Less than 2 pods on node"))
t.Fatal("Less than 2 pods on node")
Copy link
Contributor

Choose a reason for hiding this comment

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

Good to have node name as well ?

@jpayne3506 jpayne3506 force-pushed the jpayne3506/gowinrefactor branch 5 times, most recently from 980f99c to 34fcb70 Compare July 28, 2023 18:39
@jpayne3506 jpayne3506 force-pushed the jpayne3506/gowinrefactor branch from 34fcb70 to 48a7a66 Compare July 28, 2023 22:22
@jpayne3506 jpayne3506 enabled auto-merge (squash) July 28, 2023 22:22
@jpayne3506 jpayne3506 merged commit a20fb84 into Azure:master Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cni Related to CNI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants