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

feat: (IAC-1066) Add Hadolint, ShellCheck TFLint Checks, and Ansible Lint via GitHub Actions #78

Merged
merged 30 commits into from
Jul 11, 2023

Conversation

jarpat
Copy link
Contributor

@jarpat jarpat commented Jun 16, 2023

Changes

Enabled linting of the viya4-iac-k8s project with Hadolint, ShellCheck, TFLint, and Ansible Lint through a workflow in GitHub actions. Once the workflow was configured, any errors or warnings raised was resolved as part of this PR.

No change in functionality has been made with this PR mostly formatting,

Tests

Scenario Provider Base Image Cluster Version Order Cadence Notes Tasks
1 Vsphere Ubuntu 22.04 LTS v1.25.8 ****** fast:2020 Internal apply setup install -> baseline,viya,install -> viya,uninstall -> uninstall -> destroy
2 Vsphere Ubuntu 22.04 LTS v1.25.8 n/a n/a External apply setup install -> uninstall -> destroy

@jarpat jarpat marked this pull request as ready for review June 20, 2023 15:51
- role-name # roles are dynamically selected based on user's choice so we do need to use paths
- yaml[line-length] # it's easier to understand/debug the underlying command when it's not broken up, excessively long lines that make sense to split up should be caught during code review,
- name[template] # task name is being templated, this can be ignored
- command-instead-of-shell # shell should only be used when necessary, swap to command TODO future update requires functionality change
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note:
Ignoring command-instead-of-shell, command-instead-of-module, and deprecated-module linting warnings for now. To resolve these linter warnings it requires a change of functionality which I was trying to avoid with this PR. I've created a Jira ticket to address these as part of a future sprint. These are not breaking or high priority warnings that need immediate attention, just nice to have.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IAC-1088, for any internal users seeing this.

@jarpat jarpat self-assigned this Jun 20, 2023
@jarpat jarpat added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 20, 2023
Copy link
Member

@dhoucgitter dhoucgitter left a comment

Choose a reason for hiding this comment

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

LGTM

Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
playbooks/kubernetes-install.yaml Outdated Show resolved Hide resolved
playbooks/kubernetes-install.yaml Show resolved Hide resolved
Comment on lines -72 to -76
vars:
option: systemd.unified_cgroup_hierarchy
value: "1"
tags:
- install
Copy link
Member

Choose a reason for hiding this comment

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

I believe this was in place for Ubuntu 20.04 LTS has this been moved? Or if it's not needed for Ubuntu 22.04 LTS then we need to put guards on this. I have this in my own sandbox to fix this. NOTE: The version is based on when this was fixed.

  vars:
    option: systemd.unified_cgroup_hierarchy
    value: "1"
  when:
    - ansible_distribution == "Ubuntu" and ansible_distribution_version is version('21.10', '<', strict=true)
  tags:
    - install

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The git diff makes it looks like items were removed as part of the format update.
What happened on these lines was that order of keys in the task were updated to follow ansible conventions as part of this rule:
https://ansible.readthedocs.io/projects/lint/rules/key-order/

So previously the order was name>block>vars>tag and it's been updated to names>vars>tags>block

No real functionality change was made here, just the order was updated. The git diff makes it look like items were removed.
If you take a look at the code in the branch it'll be easier to see the changes https://github.com/sassoftware/viya4-iac-k8s/blob/IAC-1066/roles/kubernetes/common/tasks/main.yaml#L41-L93

roles/systems/postgres/tasks/main.yaml Show resolved Hide resolved
roles/kubernetes/common/tasks/main.yaml Show resolved Hide resolved
roles/systems/nfs_server/tasks/main.yaml Show resolved Hide resolved
@jarpat jarpat merged commit d01b360 into staging Jul 11, 2023
4 checks passed
@jarpat jarpat deleted the IAC-1066 branch July 11, 2023 14:02
@jarpat jarpat mentioned this pull request Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants