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.

Sign up for GitHub

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

Jump to bottom

Add templates for configuring Docker #105

Merged
merged 3 commits into from
Jul 6, 2018
Merged

Add templates for configuring Docker #105

merged 3 commits into from
Jul 6, 2018

Conversation

jouvin
Copy link
Contributor

@jouvin jouvin commented Oct 9, 2017

First attempt at providing templates to configure various parts of Docker service.

@jouvin jouvin added this to the 17.10 milestone Oct 9, 2017
jrha
jrha reviewed Oct 26, 2017
View reviewed changes
features/docker/backup.pan Outdated
include {'components/cron/config'};

"/software/components/cron/entries" = {
SELF[length(SELF)]=nlist(
Copy link
Member

Choose a reason for hiding this comment

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

append(SELF, ...

jrha
jrha reviewed Oct 26, 2017
View reviewed changes
features/docker/backup.pan Outdated
"perms","0755",
);

include {'components/cron/config'};
Copy link
Member

Choose a reason for hiding this comment

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

No curlies

features/docker/backup.pan Outdated

include {'components/filecopy/config'};

'/software/components/filecopy/services/{/usr/sbin/backup_docker_data}' = nlist(
Copy link
Member

Choose a reason for hiding this comment

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

dict

features/docker/backup.pan Outdated

variable DOCKER_BACKUP_FREQUENCY?="30 11 * * *";

include {'components/filecopy/config'};
Copy link
Member

Choose a reason for hiding this comment

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

No curlies

features/docker/backup.pan Outdated

variable DOCKER_BACKUP_MAIL_ERR=if(is_defined(DOCKER_BACKUP_MAIL_ERR)){"-mailerr "+SELF}else{""};

variable DOCKER_BACKUP_COMMAND?="/usr/sbin/backup_docker_data -dir "+DOCKER_BACKUP_DIR+" "+DOCKER_BACKUP_MAIL_OK+" "+DOCKER_BACKUP_MAIL_ERR;
Copy link
Member

Choose a reason for hiding this comment

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

format(...)

features/docker/core.pan Outdated
};

# Sysconfig for the docker service
include {'components/sysconfig/config'};
Copy link
Member

Choose a reason for hiding this comment

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

No curlies

features/docker/el7/config.pan Outdated


# Add package
include {'components/spma/config'};
Copy link
Member

Choose a reason for hiding this comment

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

No curlies

features/docker/el7/config.pan Outdated


# Sysconfig for the docker service
include {'components/sysconfig/config'};
Copy link
Member

Choose a reason for hiding this comment

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

No curlies

features/docker/el7/config.pan Outdated
'other_args' = DOCKER_SRV_OPTS;

#Set the docker service to be running at boot
include {'components/chkconfig/config'};
Copy link
Member

Choose a reason for hiding this comment

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

No curlies

features/docker/pipework.pan Outdated
unique template features/docker/pipework;

#Copy some needed files
include {'components/filecopy/config'};
Copy link
Member

Choose a reason for hiding this comment

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

No curlies

features/docker/pipework.pan Outdated

prefix '/software/components/filecopy/services/';

'{/usr/bin/pipework}/config' = file_contents('features/docker/pipework');
Copy link
Member

Choose a reason for hiding this comment

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

Why not put the path up with the prefix?

stdweird
stdweird reviewed Nov 29, 2017
View reviewed changes
features/docker/config.pan Outdated
SELF[length(SELF)] = format('%s', DOCKER_YUM_REPOSITORY);
repolist = SELF;
if (is_defined(DOCKER_YUM_REPOSITORY)) {
repolist = append(repolist, to_string(DOCKER_YUM_REPOSITORY));
Copy link
Member

Choose a reason for hiding this comment

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

or just append, no temp variables or anything (but i know you don't like that way of using append)

stdweird
stdweird reviewed Nov 29, 2017
View reviewed changes
features/docker/pipework Outdated
@@ -0,0 +1,248 @@
#!/bin/sh
Copy link
Member

Choose a reason for hiding this comment

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

did you write this? no license, no author (and no rpm?)

Copy link
Member

Choose a reason for hiding this comment

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

Looks a lot like an earlier version of https://github.com/jpetazzo/pipework/blob/master/pipework to me.

Copy link
Member

Choose a reason for hiding this comment

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

that's licensed under APL2.0, so should be fine to add the source and mention it in the header that is apl

@jrha
Copy link
Member

jrha commented Nov 29, 2017

@jouvin I've made changes to your branch to address my own comments, panlint and shellcheck issues.
@stdweird's question of licensing of the scripts is important and will block this being merged as we cannot redistribute software of unknown license or origin.

@jrha jrha modified the milestones: 17.12, 18.3 Dec 8, 2017
@jrha jrha modified the milestones: 18.3, 18.6 Mar 14, 2018
@jouvin jouvin self-assigned this Apr 25, 2018
@jouvin
Copy link
Contributor Author

jouvin commented May 11, 2018

I'm a bit lost about what I need to do, if anything... Is the licensing issue the only thing remaining?

@jrha
Copy link
Member

jrha commented Jun 6, 2018 •
edited
Loading

@jouvin the license is the main issue remaining, yes. It would also be good to mention what the scripts are and where updated versions can be obtained.

@jouvin
Copy link
Contributor Author

jouvin commented Jun 6, 2018

I really discovered this issue with the PR, I completely overlooked it! I don't know anything about the script causing the problem. May be @sartiran has an idea?

@sartiran
Copy link
Contributor

sartiran commented Jun 6, 2018

Hi, I've to say I've no idea of which script you are talking about. Can you give me some more details?

@jouvin
Copy link
Contributor Author

jouvin commented Jun 28, 2018

Hopefully, my last commit clarifies the licensing.

@jrha
Copy link
Member

jrha commented Jul 5, 2018

As discussed at today's standup, this still has some outstanding lint issues, if they are quickly fixed this can be merged for 18.6.

@jouvin
Copy link
Contributor Author

jouvin commented Jul 5, 2018

Yes, I completely missed how bad the state of the templates was... I just fixed it, took quite some time. They are used at GRIF&LAL, so I'm sure the contents is correct (we had a few fixes added since the PR was open). I'm waiting for the tests to see if panlint complains (I guess it will...) and I'll try to fix the remaining details later today.

@jouvin jouvin force-pushed the docker branch 2 times, most recently from 45c08ff to a58407b Compare July 5, 2018 19:37
@jouvin
Copy link
Contributor Author

jouvin commented Jul 5, 2018

panlint is happy 😄 and me too 😄

@jrha jrha merged commit c20ea97 into quattor:master Jul 6, 2018
@jrha jrha mentioned this pull request Jul 13, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stdweird stdweird stdweird left review comments

@jrha jrha jrha approved these changes

Assignees

@jouvin jouvin

Labels
None yet
Milestone
18.6
Development

Successfully merging this pull request may close these issues.
4 participants
@jouvin @jrha @sartiran @stdweird