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

GCE Cloud tests #34871

Merged
merged 10 commits into from
Jul 26, 2016
Merged

GCE Cloud tests #34871

merged 10 commits into from
Jul 26, 2016

Conversation

tonybaloney
Copy link
Contributor

@tonybaloney tonybaloney commented Jul 22, 2016

What does this PR do?

Strip out all the mocks and just focus on the certificate and import process for the GCE cloud module. The test suite needs expanding, the GCE cloud module has a ton of untested behaviours, dict mappings and all sorts of assumptions about environment and configuration that are not tested.
Change also updates all the of the temporary fixes around certificates for SuSE and MacOS/brew/ports. Libcloud 1.0 + supports certifi as the source of SSL/PEM files.

What issues does this PR fix or reference?

Issue #34852

Previous Behavior

GCE tests would hang trying to instantiate driver, GCE changed the token logic in a recent version of libcloud.

New Behavior

Remove this section if not relevant

Tests written?

Yes/No

Please review Salt's Contributing Guide for best practices.

@tonybaloney
Copy link
Contributor Author

CC @supertom @erjohnso

@tonybaloney
Copy link
Contributor Author

CC @cachedout @rallytime
p.s. I'm curious if there are integration tests how they never picked up issue #32743

@rallytime
Copy link
Contributor

@tonybaloney This doesn't look quite right. :) The only changes appear to be merge conflict lines.

As to why the test suite didn't uncover that issue, it is because the cloud integration tests (since they spin up a bunch of VMs for each cloud provider) are only run on two distributions currently: CentOS 7 and Ubuntu 14. I bet I can convince @jtand to add SUSE to that mix though.

@rallytime
Copy link
Contributor

@tonybaloney Oh, I must have been looking at an old reference before. Thanks for taking another crack at this. Can you take a look at the small pylint error, as well as the related test failure when you get a chance?

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jul 22, 2016
@tonybaloney
Copy link
Contributor Author

done and done @rallytime

@cachedout
Copy link
Contributor

@tonybaloney Looks like some merge conflicts. Could you rebase this please?

@tonybaloney
Copy link
Contributor Author

rebased.

@rallytime
Copy link
Contributor

Awesome! thanks @tonybaloney - Looks like Jenkins is having a rough time in general on our end. I'll trigger a new test run.

@rallytime
Copy link
Contributor

Go Go Jenkins!

1 similar comment
@rallytime
Copy link
Contributor

Go Go Jenkins!

@rallytime
Copy link
Contributor

Thanks for cleaning these up @tonybaloney - this looks great.

@rallytime rallytime merged commit 6c08afd into saltstack:develop Jul 26, 2016
gitebra pushed a commit to gitebra/salt that referenced this pull request Jul 26, 2016
* upstream/develop: (153 commits)
  Increase all run_script timeouts to 30s (saltstack#34956)
  Fix grains integration tests for Windows (saltstack#34941)
  GCE Cloud tests (saltstack#34871)
  Create regdata before try/except (saltstack#34931)
  Add prune option to reg.list (saltstack#34934)
  Skip inode test in Windows (saltstack#34938)
  Remove use_carrier bond for Ubuntu Xenial later (saltstack#34779)
  Add jid to salt.function orchestration events
  [bashcompletion] replace ticks with bash subshell
  [bashcompletion] ignore salt-key headers
  Add jid to orchestration returns
  Avoid UnboundLocalError in beacons module
  [bash-completion] hide timeout in salt-output
  Master performance improvement (saltstack#34916)
  Update service_rh provider to exclude XenServer >= 7. (saltstack#34915)
  Lint fixes for saltstack#34923
  pv_present should execute pvcreate with -y if existing filesystem signature detected
  Handle exception when no Slack API key was provided
  zone absent with tests
  added zone_present state check
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants