Merge ~harrykas/cloud-init:freebsd_improvement into cloud-init: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.

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)
Revision history for this message
Serhii Kharchenko (harrykas) wrote :

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

Thanks.

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)

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.

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)
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.

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.

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)
Revision history for this message
Serhii Kharchenko (harrykas) wrote :

Hello,

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

Thanks.

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
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.

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
diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py
index cec22bb..431f495 100644
--- a/cloudinit/config/cc_resizefs.py
+++ b/cloudinit/config/cc_resizefs.py
@@ -81,7 +81,7 @@ def _resize_xfs(mount_point, devpth):
8181
8282
83def _resize_ufs(mount_point, devpth):83def _resize_ufs(mount_point, devpth):
84 return ('growfs', devpth)84 return ('growfs', '-y', devpth)
8585
8686
87def _get_dumpfs_output(mount_point):87def _get_dumpfs_output(mount_point):
diff --git a/cloudinit/config/cc_set_passwords.py b/cloudinit/config/cc_set_passwords.py
index bb24d57..886f952 100755
--- a/cloudinit/config/cc_set_passwords.py
+++ b/cloudinit/config/cc_set_passwords.py
@@ -134,24 +134,50 @@ def handle(_name, cfg, cloud, log, args):
134134
135 ch_in = '\n'.join(plist_in) + '\n'135 ch_in = '\n'.join(plist_in) + '\n'
136 if users:136 if users:
137 try:137 if util.is_FreeBSD():
138 log.debug("Changing password for %s:", users)138 for line in plist_in:
139 util.subp(['chpasswd'], ch_in)139 u, p = line.split(':', 1)
140 except Exception as e:140 try:
141 errors.append(e)141 log.debug("Changing password for %s:", u)
142 util.logexc(142 util.subp(['pw', 'usermod', u, '-h', '0'], data=p)
143 log, "Failed to set passwords with chpasswd for %s", users)143 except Exception as e:
144 errors.append(e)
145 util.logexc(log,
146 "Failed to set passwords with"
147 " 'pw usermod -h 0' for %s", u)
148 else:
149 try:
150 log.debug("Changing password for %s:", users)
151 util.subp(['chpasswd'], ch_in)
152 except Exception as e:
153 errors.append(e)
154 util.logexc(log,
155 "Failed to set passwords with chpasswd"
156 " for %s", users)
144157
145 hashed_ch_in = '\n'.join(hashed_plist_in) + '\n'158 hashed_ch_in = '\n'.join(hashed_plist_in) + '\n'
146 if hashed_users:159 if hashed_users:
147 try:160 if util.is_FreeBSD():
148 log.debug("Setting hashed password for %s:", hashed_users)161 for line in hashed_plist_in:
149 util.subp(['chpasswd', '-e'], hashed_ch_in)162 u, p = line.split(':', 1)
150 except Exception as e:163 try:
151 errors.append(e)164 log.debug("Changing password for %s:", u)
152 util.logexc(165 util.subp(['pw', 'usermod', u, '-H', '0'], data=p)
153 log, "Failed to set hashed passwords with chpasswd for %s",166 except Exception as e:
154 hashed_users)167 errors.append(e)
168 util.logexc(log,
169 "Failed to set passwords with"
170 " 'pw usermod -H 0' for %s", u)
171 else:
172 try:
173 log.debug("Setting hashed password for %s:", hashed_users)
174 util.subp(['chpasswd', '-e'], hashed_ch_in)
175 except Exception as e:
176 errors.append(e)
177 util.logexc(log,
178 "Failed to set hashed passwords"
179 " with chpasswd for %s",
180 hashed_users)
155181
156 if len(randlist):182 if len(randlist):
157 blurb = ("Set the following 'random' passwords\n",183 blurb = ("Set the following 'random' passwords\n",
@@ -162,7 +188,11 @@ def handle(_name, cfg, cloud, log, args):
162 expired_users = []188 expired_users = []
163 for u in users:189 for u in users:
164 try:190 try:
165 util.subp(['passwd', '--expire', u])191 if util.is_FreeBSD():
192 util.subp(['pw', 'usermod', u, '-p', '+1s'])
193 else:
194 util.subp(['passwd', '--expire', u])
195
166 expired_users.append(u)196 expired_users.append(u)
167 except Exception as e:197 except Exception as e:
168 errors.append(e)198 errors.append(e)

Subscribers

People subscribed via source and target branches