Add yamllint checking

[obnoxxx]

This PR adds yaml lint checking to the tests and to the github CI workflows. The code is is inspired by but not identical to the code from samba-operator.

This PR is proposed to allow for easier update of our yamls, e. g. workflow yamls in future PRs.

[obnoxxx]

gitlint failed because gitlint was not found. trying to fix it now ...

[obnoxxx]

@synarete wrote:

The two commits looks perfectly fine, but as a follow-up commit there should be a fix to all those yaml files (probably using hack/yq-fixup-yamls.sh) or else make check will fail.

That's exactly what I had done initially but make check-gitlint still failed. Going over it with @phlogistonjohn , we realized that the check failure was mostly due to warnings and we fixed all or most of those and tweaked the yamllint config a but the current major problem with ci failure is that gitlint isn't even found. I am trying to fix the installtool mechanism for it, without much success so far

[phlogistonjohn]

According to the results I looked at the tool is found and does not like the length of the line:

essentially taken from corresponding code in https://github.com/samba-in-kubernetes/samba-operator

I think if you used git rebase -i and reworded that commit to add a line break right before the URL your commit would pass the check

[phlogistonjohn]

Changes generally look OK to me, although I think we'll need longer term cleanups to the makefile (not for this PR). I will approve once the commit message is fixed

[obnoxxx]

@phlogistonjohn wrote:

According to the results I looked at the tool is found and does not like the length of the line:

essentially taken from corresponding code in https://github.com/samba-in-kubernetes/samba-operator

I think if you used git rebase -i and reworded that commit to add a line break right before the URL your commit would pass the check

Thanks for pointing this out! I fixed it but my attempts to fix the installation of gitlint do not seem to have been successful: now th PR's ci check run shows the same error I see locallyt:

0s
Run make check-gitlint
env: ‘GOBIN’: No such file or directory
/home/runner/work/samba-container/samba-container/hack/install-tools.sh --gitlint /home/runner/work/samba-container/samba-container/.bin
Error: [Errno [1](https://github.com/samba-in-kubernetes/samba-container/actions/runs/5044843692/jobs/9048383923?pr=132#step:4:1)3] Permission denied: '/bin/.py'
make: *** [Makefile:3[6](https://github.com/samba-in-kubernetes/samba-container/actions/runs/5044843692/jobs/9048383923?pr=132#step:4:7)2: /home/runner/work/samba-container/samba-container/.bin/gitlint] Error 1
Error: Process completed with exit code 2

[synarete]

@phlogistonjohn wrote:

According to the results I looked at the tool is found and does not like the length of the line:

essentially taken from corresponding code in https://github.com/samba-in-kubernetes/samba-operator

I think if you used git rebase -i and reworded that commit to add a line break right before the URL your commit would pass the check

Thanks for pointing this out! I fixed it but my attempts to fix the installation of gitlint do not seem to have been successful: now th PR's ci check run shows the same error I see locallyt:

0s
Run make check-gitlint
env: ‘GOBIN’: No such file or directory
/home/runner/work/samba-container/samba-container/hack/install-tools.sh --gitlint /home/runner/work/samba-container/samba-container/.bin
Error: [Errno [1](https://github.com/samba-in-kubernetes/samba-container/actions/runs/5044843692/jobs/9048383923?pr=132#step:4:1)3] Permission denied: '/bin/.py'
make: *** [Makefile:3[6](https://github.com/samba-in-kubernetes/samba-container/actions/runs/5044843692/jobs/9048383923?pr=132#step:4:7)2: /home/runner/work/samba-container/samba-container/.bin/gitlint] Error 1
Error: Process completed with exit code 2

@obnoxxx There are few issues with opt-level Makefile, in particular when is uses and pass env and args to hack/install-tools.sh helper script. Please take a look at my fixup proposal: https://github.com/synarete/samba-container/commit/462e57182f9ea16438e27f884dd1575db5aa630a

[obnoxxx]

@phlogistonjohn wrote:

Changes generally look OK to me, although I think we'll need longer term cleanups to the makefile (not for this PR). I will approve once the commit message is fixed

I think the commit message is fixed now, but my attempt to fix the mechanism to install gitlint has failed ...

[phlogistonjohn]

You can't just copy the install-tools.sh from the other repo. First, off a lot of the tools in that script are not relevant to this repo, second I was trying to fix issues with the other script here first as a test bed so just bringing everything over wholesale undoes what I was trying to accomplish.

[obnoxxx]

@phlogistonjohn wrote:

According to the results I looked at the tool is found and does not like the length of the line:

essentially taken from corresponding code in https://github.com/samba-in-kubernetes/samba-operator

I think if you used git rebase -i and reworded that commit to add a line break right before the URL your commit would pass the check

Thanks for pointing this out! I fixed it but my attempts to fix the installation of gitlint do not seem to have been successful: now th PR's ci check run shows the same error I see locallyt:

0s
Run make check-gitlint
env: ‘GOBIN’: No such file or directory
/home/runner/work/samba-container/samba-container/hack/install-tools.sh --gitlint /home/runner/work/samba-container/samba-container/.bin
Error: [Errno [1](https://github.com/samba-in-kubernetes/samba-container/actions/runs/5044843692/jobs/9048383923?pr=132#step:4:1)3] Permission denied: '/bin/.py'
make: *** [Makefile:3[6](https://github.com/samba-in-kubernetes/samba-container/actions/runs/5044843692/jobs/9048383923?pr=132#step:4:7)2: /home/runner/work/samba-container/samba-container/.bin/gitlint] Error 1
Error: Process completed with exit code 2

@synarete wrote: @obnoxxx There are few issues with opt-level Makefile, in particular when is uses and pass env and args to hack/install-tools.sh helper script. Please take a look at my fixup proposal: synarete@462e571

Thanks! I took that commit and squashed it with mine. Now it is failing differently. looking into it now ...

[obnoxxx]

updated the PR so that things work locally for me. now there is a gitlint invocation error here in ci ... @phlogistonjohn , @synarete

[synarete]

updated the PR so that things work locally for me. now there is a gitlint invocation error here in ci ... @phlogistonjohn , @synarete

@obnoxxx It appears that your branch is not rebased over origin/master which in turn causes https://github.com/obnoxxx/samba-container/blob/add-yamllint/Makefile#L279 to fail. Don't know why it happens only in CI.

[obnoxxx]

https://github.com/obnoxxx/samba-container/blob/add-yamllint/Makefile#L279

updated the PR so that things work locally for me. now there is a gitlint invocation error here in ci ... @phlogistonjohn , @synarete

@synarete wrote:

@obnoxxx It appears that your branch is not rebased over origin/master which in turn causes https://github.com/obnoxxx/samba-container/blob/add-yamllint/Makefile#L279 to fail. Don't know why it happens only in CI.

i rebased again. now it is certainly based on origin/master. and I don't see how the error could be due to that: it looks more like an invocation syntax error.

[phlogistonjohn]

The problem is the new line:

check: check-shell-scripts check-yaml check-gitlint

Remove check-gitlint from this line and it will stop failing. The issue is that github doesn't normally pull the the various branches and tags in a repo when using the standard github action for checkout. The check-commits action uses additional steps to ensure all the needed branches are present. Therefore the check-gitlint rule must not be run by default by check

[obnoxxx]

@phlogistonjohn wrote:

The problem is the new line:

check: check-shell-scripts check-yaml check-gitlint

Remove check-gitlint from this line and it will stop failing. The issue is that github doesn't normally pull the the various branches and tags in a repo when using the standard github action for checkout. The check-commits action uses additional steps to ensure all the needed branches are present. Therefore the check-gitlint rule must not be run by default by check

Thanks for the analysis! I undid that line change and now it stopped failing indeed.

FWIW, I did not really mean to suggest to merge the PR with all included commits. instead, I offer to squash a few commits, specifically these:

• https://github.com/obnoxxx/samba-container/commit/cc479df1ec6a500273a96258fa3a5efde34b6d1d
• https://github.com/obnoxxx/samba-container/commit/b71692360b8c795e380a7c3b5ca1f3a2f4a6e127
• https://github.com/obnoxxx/samba-container/commit/e76a981cbd2d99010ac794bcff3e1b250332cae9

[obnoxxx]

updated once more to fix gitlint errors

[obnoxxx]

@mergifyio refresh

[mergify[bot]]

refresh

✅ Pull request refreshed

[obnoxxx]

@mergifyio rebase

[mergify[bot]]

rebase

✅ Branch has been successfully rebased

[obnoxxx]

@anoopcs9 - thanks for approval! and please ignore my renewed review request: it was by mistake. ... :-)

[obnoxxx]

Strangely enough, est-nightly-ad-server-kubernetes failed again in this PR ...