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

Replace the existing filesystem plugin on Linux and Darwin with filesystem_v2 #974

Merged
merged 4 commits into from
Apr 4, 2017

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Apr 3, 2017

Filesystem’s implementation leaves a lot to be desired and contains bugs that continually burn new users. This change removes the legacy filesystem plugin on Linux and changes the filesystem_v2 plugin to write to both node[‘filesystem’] and node[‘filesystem_v2’]

Signed-off-by: Tim Smith tsmith@chef.io

Filesystem’s implementation leaves a lot to be desired and contains bugs that continually burn new users. This change removes the legacy filesystem plugin on Linux and changes the filesystem_v2 plugin to write to both node[‘filesystem’] and node[‘filesystem_v2’]

Signed-off-by: Tim Smith <tsmith@chef.io>
Signed-off-by: Tim Smith <tsmith@chef.io>
it "sets both filesystem and filesystem_v2 attributes" do
plugin.run
expect(plugin[:filesystem]).to_not be_nil
expect(plugin[:filesystem_v2]).to_not be_nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expect(plugin[:filesystem]).to eq(plugin[:filesystem_v2]) as well maybe?

but this test also seems to be failing and filesystem_v2 seems to be nil for some reason...

@jaymzh
Copy link
Collaborator

jaymzh commented Apr 3, 2017

It seems confusing to me to go this way rather than renaming filesystem2 to filesystem and having it populate filesystem2 as a back-compat thing for now.

That said as the author of fs2, I'm 👍 in general. :)

@tas50
Copy link
Contributor Author

tas50 commented Apr 4, 2017

@jaymzh The end goal for this plugin and the cloud_v2 plugin is for them to just entirely assume the namespace of the existing plugins. We'll write to both locations with the new data through the Chef 13 release cycle. In Chef 14, the V2 namespaces go away and these just become the one and only filesystem and cloud plugins. It's just a bit staggered to lesson the impact as much as we can. Until we do the final cutover there's not a huge need to rename the plugin file itself.

Signed-off-by: Tim Smith <tsmith@chef.io>
@tas50
Copy link
Contributor Author

tas50 commented Apr 4, 2017

@jaymzh I take the above statement back. We should rename them so the enable/disable config makes sense.

@jaymzh
Copy link
Collaborator

jaymzh commented Apr 4, 2017

My thoughts exactly. I like this... my only comment left is that the title here says it's for Linux, but it's for Darwin too.. Also, do we need doc changes here?

@thommay thommay changed the title Replace the existing filesystem plugin on Linux with filesystem_v2 Replace the existing filesystem plugin on Linux and Darwin with filesystem_v2 Apr 4, 2017
@tas50
Copy link
Contributor Author

tas50 commented Apr 4, 2017

This definitely goes into the upgrade guide. It's a big change but will eliminate a lot of support issues when people use the old plugin and have a bad time. It's worth the pain of forcing a migration

@tas50 tas50 merged commit be53f46 into master Apr 4, 2017
@jaymzh
Copy link
Collaborator

jaymzh commented Apr 4, 2017

🥇

@tas50
Copy link
Contributor Author

tas50 commented Apr 4, 2017

Thanks again for writing this in the first place @jaymzh. Making it the default is going to prevent some 😢 out of box

@tas50 tas50 deleted the filesystem_2_no_more branch April 4, 2017 23:20
@tas50 tas50 added the Type: Enhancement Adds new functionality. label Apr 11, 2017
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Enhancement Adds new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants