Add CentOS based minion to environment in docker #124 #125

dawidmalina · 2019-01-01T17:16:36Z

No description provided.

erwindon · 2019-01-01T20:59:37Z

@dawidmalina I have a question on this PR:
It contains other changes than just adding one more machine. These changes are likely improvements, but it is now unclear whether they are related or not, since these are in a single commit.
Can you split the commit in (at least) these separate parts?

dawidmalina · 2019-01-01T21:16:56Z

Sure I will modify this :) It's just my habit as in my organisation we need to have commit per ticket (even if inside the commit you have few tasks to finished the feature) as all info must be inside the ticket. In addition I would like to validate and fix dockerfiles based on https://github.com/projectatomic/dockerfile_lint

erwindon · 2019-01-01T21:36:12Z

In my organisation we need a branch per change request (unless it is known to be a single commit) and the branch-names and commit-descriptions must always start with the ticket number. Many tools transform this into an actual URL directly into the ticketing system for easy lookup.
The validation sounds great.

erwindon · 2019-01-01T23:29:36Z

Please update runtests.sh to say that there are now 3 minions.
I've updated the 4 Dockerfiles to state MAINTAINER Erwin Dondorp <saltgui@dondorp.com>, can you do the same?
All 4 docker images are now build.
All 4 docker images are now pushed to docker-hub.
sh runtests.sh runs fine with 3 minions. Can you repeat that?

…tomic/dockerfile_lint

dawidmalina · 2019-01-01T23:56:44Z

# Analyzing dockerfile-saltmaster

Check passed!

# Analyzing dockerfile-saltminion-debian

Check passed!

# Analyzing dockerfile-saltminion-ubuntu

Check passed!

# Analyzing dockerfile-saltminion-centos


--------INFO---------

Line 22: -> RUN yum install epel-release curl gnupg2 net-tools --assumeyes   && rpm --import https://repo.saltstack.com/py3/redhat/7/x86_64/latest/SALTSTACK-GPG-KEY.pub   && echo -e "[salt-py3-latest]\nname=SaltStack Latest Release Channel Python 3 for RHEL/Centos \$releasever\nbaseurl=https://repo.saltstack.com/py3/redhat/7/\$basearch/latest\nenabled=1\ngpgcheck=1\ngpgkey=https://repo.saltstack.com/py3/redhat/7/\$basearch/latest/SALTSTACK-GPG-KEY.pub\n" > /etc/yum.repos.d/salt-py3-latest.repo   && yum update --assumeyes   && yum install salt-minion --assumeyes   && rpm -qa | grep salt-   && rm -rf /var/lib/yum/* /var/cache/yum   && yum clean all
INFO: updating the entire base image may add unnecessary size to the container. update the entire base image may add unnecessary size to the container.
Reference -> http://docs.projectatomic.io/container-best-practices/#_clear_packaging_caches_and_temporary_package_downloads

Issue for this false warning: projectatomic/dockerfile_lint#83

dawidmalina · 2019-01-02T00:05:20Z

Output of the tests with my locally build images:

yarn install v1.12.3
[1/4] 🔍  Resolving packages...
success Already up-to-date.
✨  Done in 0.31s.
yarn run v1.12.3
$ eslint saltgui/static/scripts tests/
✨  Done in 1.46s.
yarn run v1.12.3
$ stylelint 'saltgui/static/stylesheets/*.css'
✨  Done in 1.24s.
Creating docker_saltmaster-local_1_516f0dbab2d4 ... done
Creating docker_saltminion-centos_1_566be307aaa2 ... done
Creating docker_saltminion-debian_1_a95eb80235bc ... done
Creating docker_saltminion-ubuntu_1_a76110de35a8 ... done
yarn run v1.12.3
$ node tests/helpers/wait-for-docker.js
waiting for docker setup to be ready
docker setup is ready
✨  Done in 0.43s.
yarn run v1.12.3
$ mocha --trace-warnings --check-leaks --reporter spec tests/unit/ tests/functional/


  Unittests for output.js
    ✓ test formatJSON
    ✓ test isDocumentationOutput
    ✓ test isDocuKeyMatch
    ✓ test reduceDocumentationOutput
    ✓ test documentation external link conversion

  Unittests for parsecmdline.js
    ✓ test parseCommandLine

  Unittests for utils.js
    ✓ test elapsedToString with valid values
TypeError: date.getTime is not a function
    at Object.window.elapsedToString (/Users/malina/WORKSPACE/GITHUB/SaltGUI/saltgui/static/scripts/utils.js:3:65)
    at Context.done (/Users/malina/WORKSPACE/GITHUB/SaltGUI/tests/unit/utils.js:60:21)
    at callFnAsync (/Users/malina/WORKSPACE/GITHUB/SaltGUI/node_modules/mocha/lib/runnable.js:400:21)
    at Test.Runnable.run (/Users/malina/WORKSPACE/GITHUB/SaltGUI/node_modules/mocha/lib/runnable.js:342:7)
    at Runner.runTest (/Users/malina/WORKSPACE/GITHUB/SaltGUI/node_modules/mocha/lib/runner.js:455:10)
    at /Users/malina/WORKSPACE/GITHUB/SaltGUI/node_modules/mocha/lib/runner.js:573:12
    at next (/Users/malina/WORKSPACE/GITHUB/SaltGUI/node_modules/mocha/lib/runner.js:369:14)
    at /Users/malina/WORKSPACE/GITHUB/SaltGUI/node_modules/mocha/lib/runner.js:379:7
    at next (/Users/malina/WORKSPACE/GITHUB/SaltGUI/node_modules/mocha/lib/runner.js:303:14)
    at Immediate._onImmediate (/Users/malina/WORKSPACE/GITHUB/SaltGUI/node_modules/mocha/lib/runner.js:347:5)
    at processImmediate (timers.js:632:19)
    ✓ test elapsedToString with invalid values
    ✓ test makeTargetText

  Funtional tests
    Login and logout
      ✓ we should be redirected to the login page (2116ms)
      ✓ we cannot login with false credentials (11649ms)
      ✓ valid credentials will redirect us to the homepage and hide the loginform (1755ms)
      ✓ check that we can logout (2068ms)


  13 passing (18s)

✨  Done in 18.14s.
Stopping docker_saltminion-ubuntu_1_71b06f7f3282 ... done
Stopping docker_saltminion-debian_1_e1db2f7997df ... done
Stopping docker_saltminion-centos_1_f36a4455a3cc ... done
Stopping docker_saltmaster-local_1_cef50a08117a  ... done
Going to remove docker_saltminion-ubuntu_1_71b06f7f3282, docker_saltminion-debian_1_e1db2f7997df, docker_saltminion-centos_1_f36a4455a3cc, docker_saltmaster-local_1_cef50a08117a
Removing docker_saltminion-ubuntu_1_71b06f7f3282 ... done
Removing docker_saltminion-debian_1_e1db2f7997df ... done
Removing docker_saltminion-centos_1_f36a4455a3cc ... done
Removing docker_saltmaster-local_1_cef50a08117a  ... done

erwindon · 2019-01-02T23:43:21Z

Thx for updating the Dockerfiles!
Last minor things...
Can you remove the LABEL Version lines? I think that using salt version numbers is not the right thing here. Are labels Version and Name actually required?
The LABEL project is a nice addition. But can it be more precise? e.g. SaltGUI (github) or SaltGUI@github
Can you fix the spelling error in dumy expose instruction? What is the purpose of these dummy port settings?

dawidmalina · 2019-01-03T00:03:51Z

Yes, they are suggested by https//github.com/projectatomic/dockerfile_lint:

# Analyzing dockerfile-saltminion-ubuntu


--------ERRORS---------

ERROR: Required LABEL name/key 'Name' is not defined.
Reference -> http://docs.projectatomic.io/container-best-practices/#_recommended_labels_for_your_project


ERROR: Required LABEL name/key 'Version' is not defined.
Reference -> http://docs.projectatomic.io/container-best-practices/#_recommended_labels_for_your_project



--------INFO---------

INFO: There is no 'EXPOSE' instruction. Without exposed ports how will the service of the container be accessed?.
Reference -> https://docs.docker.com/engine/reference/builder/#expose

I will fix the LABEL project value as you suggested. I can also make use of salt version as (to be honest) using latest is not always proper approach and we should control what version we are building. Unless you think it will be more valuable to have there SaltGUI version which will be updated during releasing.

dawidmalina · 2019-01-03T00:23:04Z

I had just second thought. Tool is for us and not we for the tool ;) I suggest to accept not fully covered docker_lint and remove instructions which not give us any benefits and creates unneeded noise in dockerfile. What do you think?

erwindon · 2019-01-03T00:25:58Z

Name/Version --> ok, thx, I missed that. Note that both are in lowercase in that description.
Version --> yes, indeed, we should have a more stable connection to the salt version. That means we would then never rely on latest and these salt-versionnumbers should also be set in docker-compose.yml and in the installation commands for salt-master and salt-api?

dawidmalina · 2019-01-03T00:37:22Z

Let it be :) I will add following labels:

LABEL name=SaltGUI
LABEL project="https://github.com/erwindon/SaltGUI"
LABEL version=1.7.0

I propose remove ENV REFRESHED_ON and introduce ENV SALT_VERSION=2018.3.3 which will be used to install exact salt version during build.

I will also extend README.md page to points that docker will be tagged with salt version during build and extend also docker compose.

erwindon · 2019-01-03T00:38:12Z

I suggest to accept not fully covered docker_lint and remove instructions which not give us any benefits and creates unneeded noise in dockerfile. What do you think?

yes, this was one-time exercise, therefore we take only the agreed good advices and modernisations.
it is different for "jslint" and "stylelint", these run every time

dawidmalina · 2019-01-03T01:02:16Z

What do you think? All tests are passing :)

erwindon · 2019-01-03T08:32:37Z

LABEL version has now become a maintenance nightmare. And the dockerimages do not actually contain saltgui.

dawidmalina · 2019-01-03T08:49:16Z

Good point. So we have 2 options:

remove LABEL name, LABEL project, LABEL version as images are just pure salt stack images
change value to match actual purpose LABEL name=salt-master|salt-minion, LABEL project="SaltGUI testing", LABEL version=2018.3.3 (my favourite)

Which one do you preferer?

erwindon · 2019-01-04T01:36:49Z

Sorry, missed this question...
Just go with your favourite.

erwindon · 2019-01-04T23:42:05Z

last one... can you updateruntests.sh? replace two with three.

dawidmalina · 2019-01-05T01:42:39Z

done

Add CentOS based minion to environment in docker erwindon#124

0808ab1

dawidmalina force-pushed the dockerfiles branch from 39caa4a to 0808ab1 Compare January 1, 2019 17:25

dawidmalina mentioned this pull request Jan 1, 2019

Add CentOS based minion to environment in docker #124

Closed

erwindon assigned dawidmalina Jan 1, 2019

Move config files to dedicated location

5d004a4

dawidmalina force-pushed the dockerfiles branch from 49052be to 5d004a4 Compare January 1, 2019 22:25

rename image owner from maerteijn to erwindon

6d5b2bd

dawidmalina added 2 commits January 2, 2019 00:51

apply dockerfile best practicies based on https://github.com/projecta…

9dd2a33

…tomic/dockerfile_lint

update maintainer with mail: saltgui@dondorp.com

62d42f2

erwindon added enhancement work-in-progress labels Jan 3, 2019

Use strict salt version and tune labels values

4bba43a

Unify labels values

5eaf52e

We now have three minions

ef3f04b

erwindon merged commit 4e79ad6 into erwindon:master Jan 5, 2019

dawidmalina deleted the dockerfiles branch January 26, 2019 20:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CentOS based minion to environment in docker #124 #125

Add CentOS based minion to environment in docker #124 #125

dawidmalina commented Jan 1, 2019

erwindon commented Jan 1, 2019

dawidmalina commented Jan 1, 2019

erwindon commented Jan 1, 2019

erwindon commented Jan 1, 2019

dawidmalina commented Jan 1, 2019

dawidmalina commented Jan 2, 2019

erwindon commented Jan 2, 2019

dawidmalina commented Jan 3, 2019

dawidmalina commented Jan 3, 2019

erwindon commented Jan 3, 2019

dawidmalina commented Jan 3, 2019

erwindon commented Jan 3, 2019 •

edited

Loading

dawidmalina commented Jan 3, 2019

erwindon commented Jan 3, 2019

dawidmalina commented Jan 3, 2019 •

edited

Loading

erwindon commented Jan 4, 2019

erwindon commented Jan 4, 2019

dawidmalina commented Jan 5, 2019

Add CentOS based minion to environment in docker #124 #125

Add CentOS based minion to environment in docker #124 #125

Conversation

dawidmalina commented Jan 1, 2019

erwindon commented Jan 1, 2019

dawidmalina commented Jan 1, 2019

erwindon commented Jan 1, 2019

erwindon commented Jan 1, 2019

dawidmalina commented Jan 1, 2019

dawidmalina commented Jan 2, 2019

erwindon commented Jan 2, 2019

dawidmalina commented Jan 3, 2019

dawidmalina commented Jan 3, 2019

erwindon commented Jan 3, 2019

dawidmalina commented Jan 3, 2019

erwindon commented Jan 3, 2019 • edited Loading

dawidmalina commented Jan 3, 2019

erwindon commented Jan 3, 2019

dawidmalina commented Jan 3, 2019 • edited Loading

erwindon commented Jan 4, 2019

erwindon commented Jan 4, 2019

dawidmalina commented Jan 5, 2019

erwindon commented Jan 3, 2019 •

edited

Loading

dawidmalina commented Jan 3, 2019 •

edited

Loading