Merge ~sombrafam/maas:fix-dhcp-snnipets into maas:master

Proposed by Erlon R. Cruz
Status: Merged
Approved by: Björn Tillenius
Approved revision: 8fa3f80fe60ef305e20b607d884867e80b7d9360
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~sombrafam/maas:fix-dhcp-snnipets
Merge into: maas:master
Diff against target: 105 lines (+59/-3)
2 files modified
src/maasserver/dhcp.py (+27/-3)
src/maasserver/tests/test_dhcp.py (+32/-0)
Reviewer Review Type Date Requested Status
Björn Tillenius Approve
MAAS Lander Needs Fixing
Alberto Donato Approve
Review via email: mp+391758@code.launchpad.net

Commit message

LP #1809939: dhcp snippet create fail when dhcp subnet is relayed

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix-dhcp-snnipets lp:~sombrafam/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/8483/console
COMMIT: 4f13131d8602e078f3b65f359ba007fed041b9cc

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix-dhcp-snnipets lp:~sombrafam/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/8498/console
COMMIT: 791cbbed5442b73bf1a6feee091445b62a4ae8ae

review: Needs Fixing
Revision history for this message
Erlon R. Cruz (sombrafam) wrote :

Folks, not sure why the tests above are falling. Can you post the CI logs? The tests I added are passing locally. But, I see several other failures not related to my changes.

Revision history for this message
Dan Streetman (ddstreet) wrote :

you need to run 'make format'

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix-dhcp-snnipets lp:~sombrafam/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 32bff2e22a5f436d1ab89c990d59f850a3977b74

review: Approve
Revision history for this message
Alberto Donato (ack) wrote :

Looks good, just a minor nit inline

review: Approve
Revision history for this message
Björn Tillenius (bjornt) wrote :

I think you need to move the fix to another place. See inline comment.

review: Needs Fixing
Revision history for this message
Erlon R. Cruz (sombrafam) wrote :

Hi Björn, please check m inline suggestion.

Revision history for this message
Björn Tillenius (bjornt) :
Revision history for this message
Erlon R. Cruz (sombrafam) :
Revision history for this message
Björn Tillenius (bjornt) :
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix-dhcp-snnipets lp:~sombrafam/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/9680/console
COMMIT: ad58af79e08299801c0c914a431751ee519d9613

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix-dhcp-snnipets lp:~sombrafam/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 458986983d4cbe0ba7a6d8ad291e9c8cacd00287

review: Approve
Revision history for this message
Björn Tillenius (bjornt) wrote :

Thanks for the fixes! It's looking better now. I still have some comments inline, but it's mostly about style.

I was going to ask you to write a tests for the changes in validate_dhcp_config(), but looking at the existing tests, I suspect it's more work that it's worth... so I'm going to that slip by.

review: Needs Fixing
Revision history for this message
Erlon R. Cruz (sombrafam) wrote :

Ok, good to know. I have added the filter function to the node mixin, but Im curions to nkow why we need to leave them if they are not being used anywere?

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix-dhcp-snnipets lp:~sombrafam/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/9706/console
COMMIT: 8fa3f80fe60ef305e20b607d884867e80b7d9360

review: Needs Fixing
Revision history for this message
Björn Tillenius (bjornt) wrote :

Thanks, this is ready to land now!

Even though the subnet filter methods aren't called directly, I think they are called by our filtering/search enging, which get constructs the method name as a string and gets it using getattr(). That's why it won't show up in a simple grep.

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNABLE TO START LANDING

STATUS: MISSING COMMIT MESSAGE

Revision history for this message
MAAS Lander (maas-lander) wrote :
Revision history for this message
MAAS Lander (maas-lander) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches