cloud-init

Merge ~harrykas/cloud-init:freebsd_improvement into cloud-init:master

  1. Git
  2. lp:~harrykas/cloud-init
  3. freebsd_improvement
  4. Merge into master
Proposed by Serhii Kharchenko
Status: Work in progress
Proposed branch: ~harrykas/cloud-init:freebsd_improvement
Merge into: cloud-init:master
Diff against target: 96 lines (+47/-17)
2 files modified
cloudinit/config/cc_resizefs.py (+1/-1)
cloudinit/config/cc_set_passwords.py (+46/-16)
Reviewer Review Type Date Requested Status
Scott Moser Needs Fixing
Server Team CI bot continuous-integration Approve
Review via email: mp+337761@code.launchpad.net

Commit message

cc_resizefs: FreeBSD growfs should be executed with -y parameter
cc_set_passwords: added FreeBSD support

To post a comment you must log in.
Revision history for this message
Serhii Kharchenko (harrykas) wrote :

cc_resizefs: FreeBSD growfs should be executed with -y parameter
cc_set_passwords: added FreeBSD support
Tested on FreeBSD 10.4 and 11.1 i386/amd64.

Reply
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:445c7d4078d0d9126668a282b9df25fb1515b458
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~harrykas/cloud-init/+git/cloud-init/+merge/337761/+edit-commit-message

https://jenkins.ubuntu.com/server/job/cloud-init-ci/773/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/773/rebuild

review: Needs Fixing (continuous-integration)
Reply
Revision history for this message
Serhii Kharchenko (harrykas) wrote :

Added commit message.
Please trigger rebuild, because I have no permissions.

Thanks.

Reply
Revision history for this message
Joshua Powers (powersj) wrote :

Hi Serhii, did you also take a look at the style tests, namely:

cloudinit/config/cc_set_passwords.py:146:80: E501 line too long (92 > 79 characters)
cloudinit/config/cc_set_passwords.py:154:80: E501 line too long (83 > 79 characters)
cloudinit/config/cc_set_passwords.py:167:80: E501 line too long (92 > 79 characters)
cloudinit/config/cc_set_passwords.py:175:80: E501 line too long (83 > 79 characters)

Reply
~harrykas/cloud-init:freebsd_improvement updated
8d81e99... by Serhii Kharchenko

cc_set_passwords: style fixes

Revision history for this message
Serhii Kharchenko (harrykas) wrote :

Hello Joshua,

Fixed that style issues.
I am sorry for making extra work for you.

Don't I need to resubmit proposal?

Thank you.

Reply
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:8d81e996b6f612ac9512b96c5bd8d637170d2ea4
https://jenkins.ubuntu.com/server/job/cloud-init-ci/774/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/774/rebuild

review: Needs Fixing (continuous-integration)
Reply
~harrykas/cloud-init:freebsd_improvement updated
b379cf0... by Serhii Kharchenko

cc_set_passwords: style fixes, 2nd try

Revision history for this message
Serhii Kharchenko (harrykas) wrote :

Fixed, sorry again.
I definitely need to read all the coding conventions for cloud-init.

Reply
Revision history for this message
Joshua Powers (powersj) wrote :

No worries, take a look at: https://cloudinit.readthedocs.io/en/latest/topics/hacking.html Running at least 'tox' before submitting is a good idea :)

I'll launch another run shortly.

Reply
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:b379cf0b5efb7da56372bd24b4b1b5b714534769
https://jenkins.ubuntu.com/server/job/cloud-init-ci/775/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/775/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
Serhii Kharchenko (harrykas) wrote :

Hello,

Guys, any chance to get this merged into the main tree?

Thanks.

Reply
Revision history for this message
Scott Moser (smoser) wrote :

Hi.
Thanks for the contribution, you've done a good job.

It seems like we could take the 'chpasswd' code out into two functions:

def chpasswd_linux(pwdata, hashed=False):
   cmd = ['chpasswd']
   if hashed:
      cmd.append('-e')
   util.subp(cmd, pwdata)

def chpasswd_freebsd(pwdata, hashed=False):
   for line in pwdata:
       u, p = line.split(':', 1)
       cmd = ['pw', 'usermod', u, '-h' if hashed else '-H', 0]
       try:
         util.subp...

I realize the original probably doesnt have good unit test coverage.
Adding some is always a plus.

I'd also like to see some unit tests added to the code for the chpasswd.

I can help you get started if you're wondering how to get going, just ask.

review: Needs Fixing
Reply
Revision history for this message
Scott Moser (smoser) wrote :

Hi,
please let me know if you'd like some help. Feel free to ping me in #cloud-init on Freenode.

also, growfs is now fixed in trunk.

Reply

Unmerged commits

b379cf0... by Serhii Kharchenko

cc_set_passwords: style fixes, 2nd try

8d81e99... by Serhii Kharchenko

cc_set_passwords: style fixes

445c7d4... by Serhii Kharchenko

cc_set_passwords: added FreeBSD support

08edf4f... by Serhii Kharchenko

cc_resizefs: FreeBSD growfs should be executed with -y parameter

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py
2index cec22bb..431f495 100644
3--- a/cloudinit/config/cc_resizefs.py
4+++ b/cloudinit/config/cc_resizefs.py
5@@ -81,7 +81,7 @@ def _resize_xfs(mount_point, devpth):
6
7
8 def _resize_ufs(mount_point, devpth):
9- return ('growfs', devpth)
10+ return ('growfs', '-y', devpth)
11
12
13 def _get_dumpfs_output(mount_point):
14diff --git a/cloudinit/config/cc_set_passwords.py b/cloudinit/config/cc_set_passwords.py
15index bb24d57..886f952 100755
16--- a/cloudinit/config/cc_set_passwords.py
17+++ b/cloudinit/config/cc_set_passwords.py
18@@ -134,24 +134,50 @@ def handle(_name, cfg, cloud, log, args):
19
20 ch_in = '\n'.join(plist_in) + '\n'
21 if users:
22- try:
23- log.debug("Changing password for %s:", users)
24- util.subp(['chpasswd'], ch_in)
25- except Exception as e:
26- errors.append(e)
27- util.logexc(
28- log, "Failed to set passwords with chpasswd for %s", users)
29+ if util.is_FreeBSD():
30+ for line in plist_in:
31+ u, p = line.split(':', 1)
32+ try:
33+ log.debug("Changing password for %s:", u)
34+ util.subp(['pw', 'usermod', u, '-h', '0'], data=p)
35+ except Exception as e:
36+ errors.append(e)
37+ util.logexc(log,
38+ "Failed to set passwords with"
39+ " 'pw usermod -h 0' for %s", u)
40+ else:
41+ try:
42+ log.debug("Changing password for %s:", users)
43+ util.subp(['chpasswd'], ch_in)
44+ except Exception as e:
45+ errors.append(e)
46+ util.logexc(log,
47+ "Failed to set passwords with chpasswd"
48+ " for %s", users)
49
50 hashed_ch_in = '\n'.join(hashed_plist_in) + '\n'
51 if hashed_users:
52- try:
53- log.debug("Setting hashed password for %s:", hashed_users)
54- util.subp(['chpasswd', '-e'], hashed_ch_in)
55- except Exception as e:
56- errors.append(e)
57- util.logexc(
58- log, "Failed to set hashed passwords with chpasswd for %s",
59- hashed_users)
60+ if util.is_FreeBSD():
61+ for line in hashed_plist_in:
62+ u, p = line.split(':', 1)
63+ try:
64+ log.debug("Changing password for %s:", u)
65+ util.subp(['pw', 'usermod', u, '-H', '0'], data=p)
66+ except Exception as e:
67+ errors.append(e)
68+ util.logexc(log,
69+ "Failed to set passwords with"
70+ " 'pw usermod -H 0' for %s", u)
71+ else:
72+ try:
73+ log.debug("Setting hashed password for %s:", hashed_users)
74+ util.subp(['chpasswd', '-e'], hashed_ch_in)
75+ except Exception as e:
76+ errors.append(e)
77+ util.logexc(log,
78+ "Failed to set hashed passwords"
79+ " with chpasswd for %s",
80+ hashed_users)
81
82 if len(randlist):
83 blurb = ("Set the following 'random' passwords\n",
84@@ -162,7 +188,11 @@ def handle(_name, cfg, cloud, log, args):
85 expired_users = []
86 for u in users:
87 try:
88- util.subp(['passwd', '--expire', u])
89+ if util.is_FreeBSD():
90+ util.subp(['pw', 'usermod', u, '-p', '+1s'])
91+ else:
92+ util.subp(['passwd', '--expire', u])
93+
94 expired_users.append(u)
95 except Exception as e:
96 errors.append(e)

Subscribers

People subscribed via source and target branches