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.

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

Add support for mac address source rules pt2 #337

Merged
merged 2 commits into from
Jul 26, 2014

Conversation

damjanek
Copy link

PR #334 was not complete. I've added type definition.

@apenney
Copy link
Contributor

apenney commented Apr 3, 2014

Would it be possible to add some acceptance tests for this? You can look in spec/acceptance for examples of how to test parameters.

@jeckersb
Copy link
Contributor

jeckersb commented Apr 3, 2014

I'm hitting this bug and made the same fix myself. Fortunately I checked open pull requests before submitting my own. Let me know if I can help out with this one.

@gildub
Copy link

gildub commented Apr 9, 2014

+1
This is a good catch

@apenney
Copy link
Contributor

apenney commented Apr 22, 2014

Any news on being able to add acceptance tests? you can crib existing examples out of spec/acceptance to make it easy. You can run them with rspec spec/acceptance if you have vagrant/virtualbox installed to test them.

@damjanek
Copy link
Author

I somehow missed your previous comment.
I've added acceptance test. Hope it'll be enough.

@paramite
Copy link
Contributor

This patch fixed MAC address issue for me.

@kincl
Copy link

kincl commented Apr 28, 2014

+1
Works for me.

@xbezdick
Copy link
Contributor

+1

@apenney
Copy link
Contributor

apenney commented Apr 29, 2014

I like this (and the acceptance test is fine) but I find it confusing that it's called mac_addr and not mac_source after the actual argument we're passing to iptables. How do you feel about renaming it to mac_source? I'd happily merge it.

@damjanek
Copy link
Author

Done.

@apenney
Copy link
Contributor

apenney commented May 15, 2014

Can you rebase this? I wanted to merge but I can't. :(

@damjanek
Copy link
Author

Done.

@damjanek
Copy link
Author

Any plans regarding merging this PR?

@paramite
Copy link
Contributor

+1, let's merge it. We are already using it.

@ssplatt
Copy link

ssplatt commented May 29, 2014

worked for me.

@damjanek
Copy link
Author

@apenney Do you plan to merge this?

@damjanek damjanek closed this Jun 11, 2014
@damjanek damjanek reopened this Jun 11, 2014
@jeckersb
Copy link
Contributor

Please merge this, I will give you one non-refundable virtual 🍪

@paramite
Copy link
Contributor

Or we can start doing some voodoo ritual which will make some powerful spirit merge this PR instead of @apenney :)

Jeff '2 bits' Bachtel and others added 2 commits July 23, 2014 16:53
This is necessary to parse rules generated on OpenStack Havana + Neutron + OpenVSwitch
 * Missing type definition
 * Fix failing test
 * Acceptance testing + fix parameter match
 * Renamed mac_addr to mac_source
@cristifalcas
Copy link
Contributor

I would love to see this merged also :).

hunner added a commit that referenced this pull request Jul 26, 2014
Add support for mac address source rules pt2
@hunner hunner merged commit 8d8f6ae into puppetlabs:master Jul 26, 2014
@jeckersb
Copy link
Contributor

As promised, a 🍪 for @hunner. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.