Replace embedded ansible git repos with GitRepository

[Fryguy]

@carbonin @NickLaMuro Please review

Some "known" issues for future PRs:

• GitRepository's clone is not process-safe. We need to add a clone lock around the Dir.exists? call and the actual clone.
• This does not yet use the SCM credentials. SCM credentials are tied to a ConfigurationScriptSource, but GitRepository is expected to own it's own credentials, so I have a double-ownership problem. GitRepository will have to be refactored somehow before I can handle it, hence why I want to do it in a follow up.
• The default ansible playbook consolidated thing may not update properly. (i.e. it can clone on first seed, but may not update on subsequent seed)
• The UI still has the 3 checkboxes that we are going to drop. - Remove Embedded Ansible repository unsused attributes manageiq-ui-classic#5848
• When we drop the 3 options, we need to create a data migration to remove them from the source.
• Checkouts created after running the playbook still need to be cleaned up.
• Git repos over ssh don't work just yet.

[Fryguy]

@carbonin @NickLaMuro This is ready to go.

[Fryguy]

Fixed a couple of rubocops...I'm leaving the rest because they were either already there or I don't agree with them (going to open some guides discussions on the latter)

[agrare]

Could you add a state to the workflow to cleanup the repo?

[Fryguy]

I can, but it would have to be optional based on a parameter or something. AnsiblePlaybookWorkflow was written expecting the directory to be owned outside of it (i.e. playbooks stored in the manageiq plugins themselves).

[Fryguy]

I was going to do that in a followup.

[agrare]

👍

[agrare]

LGTM

[NickLaMuro]

I think most of this makes sense. I don't think most of the comments should hold up the merge.

One general question/ask: Can you link to the migration PR that handles adding git_repository_id in this PR? I don't think I got a chance to review that one, though I think it is already merged if it is not

[NickLaMuro]

From PR description:

• The UI still has the 3 checkboxes that we are going to drop.

Related PR: ManageIQ/manageiq-ui-classic#5848

Update: Probably should be in the PR description now.

[Fryguy]

Migration PR that added git_repository_id - ManageIQ/manageiq-schema#393

[NickLaMuro]

Approving just to make it official @agrare

(though, I guess there are merge conflicts now...)

[agrare]

@Fryguy can you rebase?
Thanks Nick!

[NickLaMuro]

Put together a branch that fixes the conflicts:

master...NickLaMuro:git_repo_embedded_ansible

And explained what happened with the issue with the private method in the last commit:

c0c2c45

Can turn it into a PR if you want, or you can just use it as a reference to how to fix the conflicts.

[miq-bot]

Some comments on commits Fryguy/manageiq@d892810~...f878007

spec/models/manageiq/providers/embedded_ansible/automation_manager/job_spec.rb

• ⚠️ - 13 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

[miq-bot]

Checked commits Fryguy/manageiq@d892810~...f878007 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
11 files checked, 11 offenses detected

app/models/git_repository.rb

• ⚠️ - Line 106, Col 39 - Rails/ActiveRecordAliases - Use update! instead of update_attributes!.
• ⚠️ - Line 123, Col 33 - Rails/ActiveRecordAliases - Use update instead of update_attributes.
• ❗ - Line 8, Col 33 - Style/ColonMethodCall - Do not use :: for method calls.
• ❗ - Line 8, Col 42 - Style/PercentLiteralDelimiters - %w-literals should be delimited by [ and ].

app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb

• ⚠️ - Line 56, Col 15 - Rails/ActiveRecordAliases - Use update! instead of update_attributes!.

lib/git_worktree.rb

• ❗ - Line 40, Col 3 - Style/AccessModifierDeclarations - private should not be inlined in method definitions.
• ❗ - Line 42, Col 36 - Style/NumericPredicate - Use b.name.casecmp("#{@remote_name}/#{name}").zero? instead of b.name.casecmp("#{@remote_name}/#{name}") == 0.
• ❗ - Line 42, Col 7 - Style/NumericPredicate - Use b.name.casecmp(name).zero? instead of b.name.casecmp(name) == 0.
• ❗ - Line 62, Col 3 - Style/AccessModifierDeclarations - private should not be inlined in method definitions.
• ❗ - Line 63, Col 34 - Style/NumericPredicate - Use t.name.casecmp(name).zero? instead of t.name.casecmp(name) == 0.
• ❗ - Line 78, Col 3 - Style/AccessModifierDeclarations - private should not be inlined in method definitions.