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

feat(formula): modernize, fix bugs/travis, archive/windows support #256

Merged
merged 1 commit into from
Nov 18, 2020

Conversation

noelmcloughlin
Copy link
Member

@noelmcloughlin noelmcloughlin commented Nov 10, 2020

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

YES !!!

Related issues and/or pull requests

  • Fixes/Obsoletes

#252
#249
#243
#236
#234
#219
#202
#191
#190
#160
#95
#85
#74
#251
#253

Describe the changes you're proposing

This PR modernizes the docker formula by doing the following:

  • align to template formula
  • fix bugs as a consequence of refactoring
  • add Windows support
  • Add (basic) Docker swarm (salt module) pillar-driven support/states
  • Add saltstack dockercompose module support
  • Get rid of old legacy spagetti jinja conditionals
  • Get Travis CI (package/archive) is passing

BREAKING CHANGE: This version is not backward compatible. Update
your states and pillar data to align with the new formula structure.

  • MacOS was not tested in this PR but hopefully no regression.
  • docker.containers sls was simplified (tested manually)
  • docker.swarm support is really basic (init.sls tested manually)
  • See README and pillar.example

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

  • For Linux See Travis CI/CD

  • For Windows see below:

[19:03:01.613] [ManifestAndExistingInstallationLoader] No install path specified, looking for default installation registry key
[19:03:02.120] [Program] Existing installation found: version=49427, displayVersion=2.5.0.0, basePath=C:\Program Files\Docker\Docker, channel=stable, channelU
rl=https://desktop.docker.com/win/stable/appcast.xml
[19:03:02.286] [InstallWorkflow] Using package: res:DockerDesktop
[19:03:02.287] [InstallWorkflow] Downloading
[19:03:04.606] [InstallWorkflow] Extracting manifest
[19:03:05.173] [InstallWorkflow] Manifest found: version=49427, displayVersion=2.5.0.0, channel=stable, channelUrl=https://desktop.docker.com/win/stable/appca
st.xml
[19:03:05.174] [InstallWorkflow] Checking prerequisites
[19:03:05.654] [InstallWorkflow] Existing installation is up to date
local:
----------
          ID: docker-software-package-install-other
    Function: test.show_notification
      Result: True
     Comment: The docker package is unavailable/unselected for Windows
     Started: 02:02:57.123149
    Duration: 0.0 ms
     Changes:
----------
          ID: docker-software-docker-archive-install-other
    Function: test.show_notification
      Result: True
     Comment: The docker archive is unavailable/unselected for Windows
     Started: 02:02:57.123149
    Duration: 0.962 ms
     Changes:
----------
          ID: docker-software-desktop-download
    Function: file.managed
        Name: C:\\temp\docker\Docker-Desktop.exe
      Result: True
     Comment: unless condition is true
     Started: 02:02:57.125147
    Duration: 4328.36 ms
     Changes:
----------
          ID: docker-software-desktop-install
    Function: cmd.run
        Name: C:\\temp\docker\Docker-Desktop.exe || true
      Result: True
     Comment: Command "C:\\temp\docker\Docker-Desktop.exe || true" run
     Started: 19:03:01.453507
    Duration: 8728.144 ms
     Changes:
              ----------
              pid:
                  8320
              retcode:
                  0
              stderr:
              stdout:
----------
          ID: docker-docker-compose-package-install-other
    Function: test.show_notification
      Result: True
     Comment: The docker compose package is unavailable/unselected for Windows
     Started: 02:03:10.181651
    Duration: 0.0 ms
     Changes:
----------
          ID: docker-compose-software-binary-install-other
    Function: test.show_notification
      Result: True
     Comment: The docker-compose binary is unavailable/unselected for Windows
     Started: 02:03:10.182651
    Duration: 0.0 ms
     Changes:

Summary for local
------------
Succeeded: 6 (changed=1)
Failed:    0
------------
Total states run:     6
Total run time:  13.057 s

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

@noelmcloughlin noelmcloughlin requested a review from a team as a code owner November 10, 2020 02:28
@noelmcloughlin noelmcloughlin changed the title feat(formula): modernize structure, fix bugs/travis, archive/windows support feat(formula): modernize, fix bugs/travis, archive/windows support Nov 10, 2020
@noelmcloughlin
Copy link
Member Author

noelmcloughlin commented Nov 12, 2020

Updated the PR to fix/add some states (which were tested manually)

  • docker.containers
  • docker.containers.clean
  • docker.swarm.* basic support

The Swarm support (salt module) is really basic and pillar driven (no sanity checks). Here is docker.swarm state output.

vagrant@ubuntu1804:~$ sudo salt-call state.highstate --local
[ERROR   ] Exception during resolving address: [Errno 2] Host name lookup failure
[ERROR   ] Exception during resolving address: [Errno 2] Host name lookup failure
[ERROR   ] Exception during resolving address: [Errno 2] Host name lookup failure
[ERROR   ] Exception during resolving address: [Errno 2] Host name lookup failure
[WARNING ] The function "module.run" is using its deprecated version and will expire in version "Phosphorus".
local:
----------
          ID: docker-swarm-swarm_init
    Function: module.run
        Name: swarm.swarm_init
      Result: True
     Comment: Module function swarm.swarm_init executed
     Started: 01:50:43.014161
    Duration: 792.285 ms
     Changes:
              ----------
              ret:
                  ----------
                  Comment:
                      Docker swarm has been initialized on ubuntu1804.localdomain and the worker/manager Join token is below
                  Tokens:
                      ----------
                      Manager:
                          SWMTKN-1-48ut7q4khmm8yb9t520wmgdsj51le0ltxrzghf01lnfxa8g23o-0i6pqrrl2q2hwh1c8j8a6ya50
                      Worker:
                          SWMTKN-1-48ut7q4khmm8yb9t520wmgdsj51le0ltxrzghf01lnfxa8g23o-1jymxscoyrcfgp0m2zeg364xy

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time: 792.285 ms

@noelmcloughlin
Copy link
Member Author

Includes #251 and #253 PRs.

- align tom template formula
- fix bugs
- add Windows support
- Add saltstack dockercompose module support
- Get rid of confusing old legacy spagetti jinja conditionals
- Travis CI (package/archive) is passing
- Windows is passing
- Fixes/obsoletes: saltstack-formulas#252, saltstack-formulas#249, saltstack-formulas#243, saltstack-formulas#236, saltstack-formulas#234, saltstack-formulas#219, saltstack-formulas#202, saltstack-formulas#191
- Fixes/obsoletes: saltstack-formulas#190, saltstack-formulas#160, saltstack-formulas#95, saltstack-formulas#85, saltstack-formulas#74
- Includes saltstack-formulas#251 and saltstack-formulas#253
- Add Swarm support

BREAKING CHANGE: This version is not backwards compatible. Update
 your states and pillar data to align with new formula.

 - MacOS was not tested in this PR but hopefully no regression.
 - docker.containers: sls was simplified (raise PR if regression)
@noelmcloughlin
Copy link
Member Author

noelmcloughlin commented Nov 15, 2020

Updated commit to fix saltstack/salt#58920 (CentOS)

@noelmcloughlin
Copy link
Member Author

noelmcloughlin commented Nov 18, 2020

@myii is unavailable for review and this PR is too big to be reviewed anyway. So other measurements are needed:

  • This PR fixes many open issues.
  • Travis CI/CD is passing and test coverage is greater than before [except for service.running]
  • @BadgerOps gave a thumbs up above.
  • @exoyds is happy to close feat(compose-ng): Added networks keyword support #251
  • I tested docker. containers and docker. compose and existing docker.compose.ng states manually on Ubuntu/CentOS/Windows and they appear to be working - we cannot run those tests in Travis using docker driver.

Since it's open for 9 days and I need some of these features for current projects I will selfie-merge. Hope everyone is happy with this. The commit is marked as BREAKING so a new major semantic release will be generated. I will close fixed issues.

@saltstack-formulas-travis

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@BadgerOps
Copy link

@noelmcloughlin for posterity, I checked out the PR branch and tested it on centos 8, centos 7.3 and appeared to work just fine for installing docker and docker-compose. Happy to do some more testing if anything crops up. Thanks!

@noelmcloughlin
Copy link
Member Author

@BadgerOps Thank you. This is the first time Travis CI/CD is working so that helps.
Kitchen-Test uses Docker Driver so I think containers and compose states cannot work in CI/CD unless we use Vagrant driver? Does Docker run inside Docker?
I have not tested Salt swarm and dockercompose states but they just use Salt Modules so nothing special there.

myii added a commit to myii/ssf-formula that referenced this pull request Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants