Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Enabling 2 Windows E2E tests #3332

Merged
merged 14 commits into from
Jun 22, 2018

Conversation

PatrickLang
Copy link
Contributor

What this PR does / why we need it:

  • Adds timestamps to deployments commands in e2e tests. Some Windows deployments take a long time, so this will be helpful in tuning timeout durations
  • Reenables 2 Windows tests commented out due to flakes
  • Skip a portion of test that's failing to a known bug with DNS setup. Hopefully this will fix the test flake. If it does, I propose moving it to a separate test case focused on outbound network connectivity from Windows pods since that has been flaky over the last few weeks.

I'm relying on the automated tests for this, so there may be another commit or two added before it's ready to merge.

Special notes for your reviewer:

If applicable:

  • documentation
  • unit tests
  • tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

@PatrickLang
Copy link
Contributor Author

/assign @jackfrancis

@PatrickLang
Copy link
Contributor Author

If it passes this could replace #3241

@codecov
Copy link

codecov bot commented Jun 22, 2018

Codecov Report

Merging #3332 into master will decrease coverage by 0.54%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3332      +/-   ##
==========================================
- Coverage      53%   52.46%   -0.55%     
==========================================
  Files         104      103       -1     
  Lines       15682    15472     -210     
==========================================
- Hits         8313     8118     -195     
+ Misses       6627     6624       -3     
+ Partials      742      730      -12

@PatrickLang
Copy link
Contributor Author

/retest ?

@PatrickLang
Copy link
Contributor Author

@jackfrancis can you kick this tomorrow?

This seems like a flake unrelated to my last change

ummarizing 1 Failure:

[Fail] Azure Container Cluster using the Kubernetes Orchestrator with a linux agent pool [It] should be able to autoscale 
/go/src/github.com/Azure/acs-engine/test/e2e/kubernetes/kubernetes_test.go:493

Ran 17 of 22 Specs in 725.367 seconds
FAIL! -- 16 Passed | 1 Failed | 0 Pending | 5 Skipped --- FAIL: TestKubernetes (725.37s)

@jackfrancis
Copy link
Member

/retest

@PatrickLang
Copy link
Contributor Author

I'm kicking failed tests through CircleCI UI. k8s-linux-default-e2e passed now

)

// PrintCommand prints a command string
func PrintCommand(cmd *exec.Cmd) {
fmt.Printf("\n$ %s\n", strings.Join(cmd.Args, " "))
}

// RunAndLogCommand logs the command with a timestamp when it's run, and duration at and
func RunAndLogCommand(cmd *exec.Cmd) ([]byte, 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.

@jackfrancis @CecileRobertMichon are you ok with this tiny refactor as part of this PR? If not I can split it into a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

before

STEP: Creating a nginx deployment

$ kubectl run ingress-nginx-kubernetes-eastus-5726-17635 -n default --image library/nginx:latest --overrides { "apiVersion": "extensions/v1beta1", "spec":{"template":{"spec": {"nodeSelector":{"beta.kubernetes.io/os":"linux"}}}}} --labels=app=ingress-nginx

after

STEP: Creating a nginx deployment
2018/06/22 16:38:14 $ kubectl run nginx-kubernetes-eastus-67793-62845 -n default --image library/nginx:latest --overrides { "apiVersion": "extensions/v1beta1", "spec":{"template":{"spec": {"nodeSelector":{"beta.kubernetes.io/os":"linux"}}}}}
2018/06/22 16:38:14 #### $ kubectl run nginx-kubernetes-eastus-67793-62845 -n default --image library/nginx:latest --overrides { "apiVersion": "extensions/v1beta1", "spec":{"template":{"spec": {"nodeSelector":{"beta.kubernetes.io/os":"linux"}}}}} completed in 286.941082ms

Copy link
Contributor

Choose a reason for hiding this comment

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

Fine with me but I think there's a typo at the end of the comment

@jackfrancis
Copy link
Member

/lgtm

@acs-bot acs-bot added the lgtm label Jun 22, 2018
@acs-bot
Copy link

acs-bot commented Jun 22, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, PatrickLang

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

)

// PrintCommand prints a command string
func PrintCommand(cmd *exec.Cmd) {
fmt.Printf("\n$ %s\n", strings.Join(cmd.Args, " "))
}

// RunAndLogCommand logs the command with a timestamp when it's run, and the duration at end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @CecileRobertMichon - fixed typo

@jackfrancis jackfrancis merged commit 47dd3c9 into Azure:master Jun 22, 2018
@ghost ghost removed the in progress label Jun 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants