Workflow matrix

[obnoxxx]

convert workflow to matrix to test OS_NAME vs BUILD_ARCH and add opensuse to the matrix

[obnoxxx]

unknown flag: --arch strange! ...

[mergify[bot]]

This pull request now has conflicts with the target branch. Please resolve these conflicts and force push the updated branch.

[obnoxxx]

the remaining problem seems to be the lack of podman support of the docker-image-artifact action: https://github.com/ishworkh/docker-image-artifact/issues/3 @phlogistonjohn - @anoopcs9

[obnoxxx]

@phlogistonjohn, I rebased on top of master after #138 merged, but test still fail with cancelled operations ...

[obnoxxx]

strange. test-server and test-nightly-servers jobs fail with an error killing a container:

Container started, ID: '85a37cd165a28caae3eccab07b306917b2d847fca3a589306550fcd87016b287'
Error: No such container: 85a3[7](https://github.com/samba-in-kubernetes/samba-container/actions/runs/5380524642/jobs/9763455344?pr=136#step:4:8)cd165a2[8](https://github.com/samba-in-kubernetes/samba-container/actions/runs/5380524642/jobs/9763455344?pr=136#step:4:9)caae3eccab07b306917b2d847fca3a589306550fcd87016b287
Error listing samba shares
Error response from daemon: Cannot kill container: 85a37cd165a28caae3eccab07b306[9](https://github.com/samba-in-kubernetes/samba-container/actions/runs/5380524642/jobs/9763455344?pr=136#step:4:10)17b2d847fca3a589306550fcd87016b287: No such container: 85a37cd165a28caae3eccab07b306917b2d847fca3a589306550fcd87016b287

[obnoxxx]

trying to run make test-server locally, I get a different error:

$ make test-server
Error: lsetxattr /var/folders/hs/vmf05zhn49b22zvpjxpc0gm80000gn/T/tmp.lj2dCA6s/containers-user-501: operation not supported
Error running samba container
$ 

this might be related to podman on MacOS ...

[obnoxxx]

@mergifyio rebase

[mergify[bot]]

rebase

✅ Branch has been successfully rebased

[phlogistonjohn]

Hi @obnoxxx, as we discussed earlier today I have pushed some additional patches into your PR. All the jobs that ran are green at the moment. However, there are still a number of issues with the PR:

• There's excess whitespace in the YAML file.
• The changes to the structure of the actions trips up github's "required statuses" and it acts like some statuses have not been reported. This is despite some of them having the same name.
• Due to the above, I don't think this PR can be automerged once approved, we'll probably need to fix mergify rules and merge it manually
• I'm not entirely sure how the push target will work if we merge these matrix changes. After reading the code a few times I think we are probably OK and the push job will just rebuild everything it needs, but I am not certain.
• There are a bunch of "fixup" patches in the PR. I think I'd prefer to squash all the changes to container-image.yml into a single patch.

Now that I've commited directly to this PR, I am willing to do more, but I also want to give you a chance to make changes as needed. First, let's deal with the witespace issue. Let me know if you want me to push a change to clean up the (new) whitespace issues in the yaml file. After that, we can squash things and then get reviews before dealing with the other things.

[phlogistonjohn]

Oh, and you'll probably want to look at my current changes and provide feedback on those too. :-)

[obnoxxx]

@phlogistonjohn wrote:

Oh, and you'll probably want to look at my current changes and provide feedback on those too. :-)

The additional patches look good to me, thanks!

I don't see the whitespace problem in the yaml yet. feel free to fix it in the PR directly .

[obnoxxx]

@dmulder - FYI: this PR adds (among other things) the testing of opensuse builds to the CI. It has already uncovered one issue with the image(s): a missin /etc/nsswitch.conf file. we work around this with 2d5dacb6d403630f9c44e81232621f70f9b1eb1c. I'd appreciate if you could take a look and let us know what you think about this approach ...

[obnoxxx]

@synarete wrote:

Commit message has a type: "thetest" instead of "the test". Plus, there is redundant white space before "test-server".

thanks, I fixed those.

Consider using vim as you git-commit editor and add autocmd FileType gitcommit setlocal spell to your .vimrc. This will catch those kind of errors upon commit.

thanks for the hint. I might try that.

[dmulder]

It has already uncovered one issue with the image(s): a missin /etc/nsswitch.conf file. we work around this with 2d5dacb.

I also encountered that issue, which is the reason for the glibc dependency (it owns/provides /etc/nsswitch.conf, or did anyway). Maybe that has changed in a recent Tumbleweed snapshot.

[obnoxxx]

@dmulder wrote:

It has already uncovered one issue with the image(s): a missing /etc/nsswitch.conf file. we work around this with 2d5dacb.

I also encountered that issue, which is the reason for the glibc dependency (it owns/provides /etc/nsswitch.conf, or did anyway). Maybe that has changed in a recent Tumbleweed snapshot.

Thanks for looking at this, @dmulder. And since you are aware of the issue and the root cause, do you suggest a different/better fix or workaround?

[obnoxxx]

This PR is in a somewhat special situation because of the following chicken-and-egg problem:

• The PR adds new CI checks by expanding pe-existing checks with the newly introduced (OS)x(Arch) matrix into multiple checks, essentially removing the original non-matrix checks
• github branch protection settings configure an explicit list of required checks. For the merge button to become click-able, not only does the PR need two approving reviews but additionally, all required checks must have succeeded. The original, currently configured required checks can not pass since they are removed. but the newly added checks should only be added to the required list after the PR has been merged. otherwise, this would block any other PRs.
• similarly, in order for mergify to automatically merge the PR. the mergify config would need to be expanded by adding the new checks to the required list there. it can only be done after the checks definition has been added this PR) since adding it earlier would result in an invalid mergify config referencing nonexisting checks. this config update could* be done as part of this PR without breaking anything but for the sake of cleanliness is breferably done in a separate follow-up PR.

so since automatic merging and merge-ability is difficult to achieve or at least undesirable, the path forward seems to be to get two approvals and manually merge with adminitrative override privileges an change branch protection and mergify config after that.

[phlogistonjohn]

I'd be happy to perform the merge. We just need the approvals. Since it's already late for some of the team we may not get them today, but let's try to get them by tomorrow!