Please don't override flavor guest capability

[dustymabe]

The vagrant-service-manager plugin is modifying the "flavor" guest capability [1].

This has side effects on code (example at [2]) that assumes it is going to get back
what is returned from the actual flavor cap in vagrant core code [3].

[1] https://github.com/projectatomic/vagrant-service-manager/blob/master/plugins/guests/redhat/cap/flavor.rb
[2] https://github.com/dustymabe/vagrant-sshfs/blob/master/lib/vagrant-sshfs/cap/redhat/sshfs_client.rb#L26
[3] https://github.com/mitchellh/vagrant/blob/master/plugins/guests/redhat/cap/flavor.rb

[bexelbie]

We need to find a way to test for an adb/cdk box and not break other things. I think this was a first attempt by @navidshaikh and @budhrg - sounds like it needs a bit of a rethink.

Do you have a pointer to a better method @dustymabe

[dustymabe]

Well if the point is just to identify the cdk box and this is the only plugin that needs it then I would say we can just define a new capability (call it 'iscdk' or 'isadb' or we can have both if we want to) rather than overriding the flavor capability.

then to check it we can first check to see if it is redhat and then to see if it is cdk and then act.

[LalatenduMohanty]

@dustymabe @bexelbie I had suggested @budhrg to add flavor guest capability for ADB/CDK. This was the first attempt, so we wanted to get something to work. The ideal way would be adding a separate guest capability for CDK/ADB and then re-implement flavor for those guest capability. IMO it is perfectly fine to add new guest capability which inherits from redhat and then add new/modify methods on top of that.

[bexelbie]

We need both CDK and ADB identification - but yes. Do we need to point anyone at a model for capability adding?

[dustymabe]

IMO it is perfectly fine to add new guest capability which inherits from redhat and then add new on top of that.

@LalatenduMohanty The problem is that when you do this it overrides the behavior for everyone who was already using the redhat "flavor" guest capability. So whatever output they were expecting is now probably not going to be right (that is the issue I hit). We really shouldn't be overriding core vagrant stuff with our own unless we have a really good reason. As soon as we start breaking other plugins and other core functionality then they will toss us to the side pretty quick.

[dustymabe]

IMO it is perfectly fine to add new guest capability which inherits from redhat and then add new on top of that.

reading this again - "add new guest capability" --> do you mean one not named flavor? If so then yes, that works :)

[LalatenduMohanty]

@dustymabe I mean something like , how fedora inherits from redhat (in Vagrant code base). So we need Guest capability and name it ADB and CDK (similar to Fedora) and then implement flavor method for CDK and ADB. This way we are not changing redhat's flavor method.

[dustymabe]

@LalatenduMohanty

What will be returned when I run the following code on the cdk box:

machine.guest.capability("flavor")

If it's not :rhel7 then you are breaking plugins that depend on that. In other words if you want people to be able to use other plugins with the ADB/CDK (that they may be used to) then you don't want to break this.

[LalatenduMohanty]

@dustymabe got it. So we cant have the same method name. So I have renamed the method as os_variant in #92

[dustymabe]

Other things that use this:
https://github.com/mitchellh/vagrant/blob/master/plugins/guests/redhat/cap/change_host_name.rb#L20
https://github.com/mitchellh/vagrant/blob/master/plugins/guests/redhat/cap/configure_networks.rb#L16

This could really be breaking things and causing other issues that you guys might be trying to chase down. Essentially this is going to make things happen via "rhel6" mechanisms, not rhel7.

[dustymabe]

@LalatenduMohanty +1 :)

[hferentschik]

FYI, this broke also my serverspec testsuite I have to verify the CDK releases. Test were failing in very odd ways. If I would not have heard about this discussion, I would have been completely lost (even though it took still a while to make the connection :-()

Seems service-manager 0.0.3 is seriously broken with this issue and issue #95. Do we have a ETA for a 0.0.4.