Feature/v4 certify#302
Feature/v4 certify#302steven-schattenberg-itential wants to merge 8 commits intoitential:feature/v4from
Conversation
steven-schattenberg-itential
requested review from
Amunagala-itential,
Madias2222,
Nick-Andreano,
bill-minear and
kvelarde-itential
|
Can you update the PR title to something more descriptive? |
There was a problem hiding this comment.
Are the local variables in this file showing naming warnings when ansible-lint is run?
There was a problem hiding this comment.
Yes, they are:
Variables names from within roles should use common_ as a prefix. (vars: component_name)
Variables names from within roles should use common_ as a prefix. (vars: hw_specs_var_name)
But putting "common" in front of them will give them strange semantics. I can add that if you like but I think it will be wierd.
There was a problem hiding this comment.
I agree. Prepending "common" reads a little weird. I'm fine with leaving them as-is. We will have to address this anyway when we split out the roles in v5.
roles/common/tasks/verify-host.yml
Outdated
| msg: "{{ host_info }}" | ||
|
|
||
| # Check for proxy | ||
| - name: Check environment variables |
There was a problem hiding this comment.
| - name: Check environment variables | |
| - name: Check for proxy settings in environment variables |
There was a problem hiding this comment.
What user are these command going to run as? Do we need to run these as the itential user?
There was a problem hiding this comment.
What user are these command going to run as?
root
Do we need to run these as the itential user?
I'm not exactly sure.
There was a problem hiding this comment.
What is the intention of this check? Is it to call out that during the install process all the external calls (dnf, pip, etc) will be going through a proxy?
2 participants
Consolidate common code into a shared file. Create space for component specific verification tasks. Added checks for proxy settings.