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

Vagrant 2.1.4 breaks require, replace with load #371

Closed
electriquo opened this issue Aug 31, 2018 · 18 comments
Closed

Vagrant 2.1.4 breaks require, replace with load #371

electriquo opened this issue Aug 31, 2018 · 18 comments

Comments

@electriquo
Copy link

electriquo commented Aug 31, 2018

i am using vagrantfiles as follows

platforms:
    driver:
      vagrantfiles:
        - Vagrantfile_extension.rb
$ cat Vagrantfile_extension.rb
Vagrant.configure('2') do |config|
  config.vagrant.plugins = ['vagrant-disksize']
  config.disksize.size = '50GB'
end

when i execute kitchen converge, vagrant-disksize is not invoked.
i used the flowing command to launch vagrant directly with the Vagrantfile generated by kitchen.

$ VAGRANT_VAGRANTFILE=.kitchen/kitchen-vagrant/foo-ubuntu-1604/Vagrantfile vagrant up

but again, vagrant-diskzie was not invoked.
i modified the Vagrantfile generated by kitchen, and replaced require with load and then vagrant-disksize was invoked.

some more technical information

$ vagrant --version
Vagrant 2.1.4

$ bundle exec kitchen --version
Test Kitchen version 1.23.2

$ cat Gemfile.lock | grep 'kitchen-vagrant ('
    kitchen-vagrant (1.3.3)

shall i file a bug?
can someone confirm this issue?

@cheeseplus
Copy link
Contributor

cheeseplus commented Sep 1, 2018

Based on the formatting of the snippet, the indentation is off which means YAML won't see the key in the right place.

platforms:
  driver:
    vagrantfiles:
    - Vagrantfile_extension.rb

Please try checking the indentation first, if that's still a problem we can investigate. Also, kitchen diagnose --all will show you the merged configs so you can see what values are getting set or not set.

@electriquo
Copy link
Author

@cheeseplus:
i followed the indentation per the documentation. in both cases, that is with placing double space before the dash and without double space, kitchen diagnose shows that Vagrantfile_extension.rb is included.

there is no issue with kitchen picking up Vagrantfile_extension.rb, but the require instruction is the issue:

i modified the Vagrantfile generated by kitchen, and replaced require with load and then vagrant-disksize was invoked.

@cheeseplus
Copy link
Contributor

I'm not sure when it came to be that load was required over require - it's been there like that for the last 4 years and it's not often used so not entirely surprised. I can't find much guidance here from upstream Vagrant so we could look at a PR for changing the keyword there to load

@electriquo
Copy link
Author

@cheeseplus:
i can confirm that it used to work in the past. i noticed the problem since vagrant 2.1.4 released 3 days ago.
can you please try to reproduce and confirm whether you are experiencing the same issue?

@cheeseplus
Copy link
Contributor

I will when it's not a holiday weekend.

@cheeseplus
Copy link
Contributor

This seems like a change of behaviour in upstream Vagrant, I haven't found any specific notes or commits related to the issue but we can change the key as needed if it works with older versions as well.

@electriquo
Copy link
Author

@cheeseplus: thank you for the reply. sadly, i cannot test it with old vagrant versions to check for backward compatibility. what do you advise?

@cheeseplus
Copy link
Contributor

cheeseplus commented Sep 6, 2018

Confirmed the existing code works fine with Vagrant 2.1.2

Also confirmed that changing it to load does seem to properly load it in 2.1.4.

@electriquo
Copy link
Author

@cheeseplus: thank you for the confirmation. will you push the code change to fix it?

@cheeseplus
Copy link
Contributor

Further testing needs to happen to ensure that I don't break earlier versions by changing to load. If this is important for you then I suggest rolling back your Vagrant version until such time as we can resolve it. It's not clear yet whether this was an intentional change on the part of the Vagrant team so I don't want to wildly flip switches when it's easy enough for users to stick on 2.1.3 or below.

@electriquo
Copy link
Author

electriquo commented Sep 6, 2018

@cheeseplus:

Pull requests are very welcome! Make sure your patches are well tested.

i would like to assist in pushing it. this is how i thought of doing it... :)
if for every vagrant version all the tests will pass (executing rake test), is it enough?

@alval5280
Copy link

FWIW I've verified this behavior as well, and have "solved" by downgrading to Vagrant 2.1.2. I would like to see s/require/load/ for kitchen-vagrant moving forward.

@electriquo
Copy link
Author

i completed the tests against all vagrant version but i do not know whether it is enough...

@cheeseplus cheeseplus changed the title vagrantfiles are not honored Vagrant 2.1.4 breaks require, replace with load Sep 10, 2018
cheeseplus pushed a commit that referenced this issue Sep 11, 2018
Signed-off-by: Seth Thomas <[email protected]>
cheeseplus pushed a commit that referenced this issue Sep 11, 2018
Signed-off-by: Seth Thomas <[email protected]>
cheeseplus pushed a commit that referenced this issue Sep 11, 2018
@electriquo
Copy link
Author

electriquo commented Sep 11, 2018

i see the merge of the pull request, but a new version has yet have been published to rubygems.
excuse my lame question, any idea when we will be able to enjoy your yield? :)

@cheeseplus
Copy link
Contributor

I have a few other fixes that I am attempting to get in prior to release, if you desperately need this you can modify your local install files in the case of ChefDK or install the gem from source.

@electriquo
Copy link
Author

electriquo commented Sep 11, 2018

@cheeseplus: thank you. why won't push incrementally?

@cheeseplus
Copy link
Contributor

Because there are other fixes that should go in with the release. It's been 6 hours since I merged this change - please be patient.

@cheeseplus
Copy link
Contributor

1.3.4 has been released to RubyGems

stissot pushed a commit to Ecodev/kitchen-vagrant that referenced this issue Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants