jeremymv2/purge users #92
Conversation
lib/chef/knife/ec_base.rb
Outdated
| end | ||
|
|
||
| def remote_user_list | ||
| @remote_user_list ||= remote_users.keys.map(&:to_sym) |
There was a problem hiding this comment.
Why are we calling to_sym here since we are going to to_s them before yielding below?
There was a problem hiding this comment.
You're right - the speed savings aren't there. I changed to an array of strings.
lib/chef/knife/ec_backup.rb
Outdated
| for_each_user_purge do |user| | ||
| ui.msg "Deleting user #{user} from local backup (purge is on)" | ||
| begin | ||
| File.delete("#{dest_dir}/users/#{user}.json") |
There was a problem hiding this comment.
Just in case you might want ::File since we are inside Chef here.
There was a problem hiding this comment.
Changed to ::File
lib/chef/knife/ec_base.rb
Outdated
| end | ||
|
|
||
| def for_each_user_purge | ||
| purge_list = if opt_parser.default_argv[1] == 'backup' |
There was a problem hiding this comment.
This could be problematic if this code was ever called from a different context. I'm wondering if it might be cleaner to have a users_for_purge function that could be implemented in the Backup and Restore classes (with an implementation here that raises an Unimplemented error. I don't usually like that kind of OOP indirection but it feels a bit safe than looking at argv
There was a problem hiding this comment.
I moved the implementation to each backup|restore class as well as in base where it raises an Chef::Knife::EcBase::UnImplemented error.
Signed-off-by: Jeremy J. Miller <[email protected]>
Signed-off-by: Jeremy J. Miller <[email protected]>
Signed-off-by: Jeremy J. Miller <[email protected]>
Signed-off-by: Jeremy J. Miller <[email protected]>
Signed-off-by: Jeremy J. Miller <[email protected]>
Signed-off-by: Jeremy J. Miller <[email protected]>
Signed-off-by: Jeremy J. Miller <[email protected]>
Signed-off-by: Jeremy J. Miller <[email protected]>
Signed-off-by: Jeremy J. Miller <[email protected]>
Signed-off-by: Jeremy J. Miller <[email protected]>
jeremymv2
force-pushed
the
jeremymv2/purge_users
branch
from
e60d352 to
33d37dc
Compare
|
Hi @stevendanna. Thanks for the initial review. Could you take a look at the modifications I made since? |
I noticed while performing backup & restores that user deletions are not purged when using the
purgeoption.purgewas not implemented inknife-ec-backuponly inChefFS- users and user_acls are handled outside ofChefFS, inknife-ec-backup.If we want user deletions to sync, this change adds the
purgecapability for users and user_acls inknife-ec-backup.It requires #87 to be merged first to add the
purgeoption.