tests: Lift exclusive: false on tests that actually mutate system

[cgwalters]

I'm planning a PR to coreos-assembler to inject
ProtectSystem=strict on tests that say exclusive: false
to actively enforce their read only nature.

These tests do mutate the system, and are hence not exclusive.

[dustymabe]

exclusive: false doesn't mean the tests are read-only. It simply means the tests can be run along with other tests on the same system and they don't conflict. If there are two exlusive: false tests that conflict then they can name that they conflict with each other and they will be run in two separate "non-exclusive-tests" buckets (two separate VMs)

[cgwalters]

I'd say the tag means what we choose it to mean.

If there are two exlusive: false tests that conflict then they can name that they conflict with each other and they will be run in two separate "non-exclusive-tests" buckets (two separate VMs)

Ah...wow really? OK yes I see that documented, but AFAICS it hasn't been used? It seems very fragile to me to try to do that, because it requires global knowledge.

Let's take the security.passwd test that creates a new unprivileged user. Is that likely to conflict with any other concurrently running test? Very likely not...unless someone went to add a test that verified there was just the core user in a default setup (which would be a potentially reasonable thing to do).

The kubernetes.kube-watch test? Probably not as is today, but if it's OK to create touch /etc/kubernetes/kubeconfig in a non-exclusive test, I can totally imagine some other cri-o or podman test coming along and accidentally trying to parse it...

I think my core argument here is: it's not worth the potential for obscure bugs with the current definition of exclusive: false.

[dustymabe]

I'd say the tag means what we choose it to mean.

What I described above is the meaning behind it today.

If there are two exlusive: false tests that conflict then they can name that they conflict with each other and they will be run in two separate "non-exclusive-tests" buckets (two separate VMs)

Ah...wow really? OK yes I see that documented, but AFAICS it hasn't been used? It seems very fragile to me to try to do that, because it requires global knowledge.

No it's not used today, but that's mostly because we haven't combed through our tests more. We still have an action item for that: #1229

Also I don't really know what you mean by global knowledge. All you need to know is if two tests conflict, which you can easily find out by running the test suite. Then test A names test B as a conflict (or vice versa, or both) and never have the problem again.

Let's take the security.passwd test that creates a new unprivileged user. Is that likely to conflict with any other concurrently running test? Very likely not...unless someone went to add a test that verified there was just the core user in a default setup (which would be a potentially reasonable thing to do).

The kubernetes.kube-watch test? Probably not as is today, but if it's OK to create touch /etc/kubernetes/kubeconfig in a non-exclusive test, I can totally imagine some other cri-o or podman test coming along and accidentally trying to parse it...

I think my core argument here is: it's not worth the potential for obscure bugs with the current definition of exclusive: false.

We haven't had major problems with it.

[bgilbert]

Kicking tests out of exclusive: false isn't free: it causes the test suite to launch more VMs, making tests incrementally more painful to run. It seems to me that if a non-exclusive test mutates state, intentionally or not, that isn't actually a big deal as long as the tests are still functioning as intended. You're right that obscure test bugs are possible, but they don't affect users, and we'll see the failures and can fix them. Has this been a problem in practice?

[travier]

Wasn't the idea behind those non exclusive test to be able to split to single ro test into multiple ones without a significant cost?
I agree that we want to reduce the cost of the tests as much as possible and at the same time I think we should be explicit when we want non RO tests to share a VM.

[cgwalters]

OK instead of trying to change the meaning of exclusive for now, coreos/coreos-assembler#3060 will allow us to supersede most of its uses, without breaking the tests that use it now.

I'll do a separate PR to switch over tests to use isolation; let's debate overall plan in the cosa PR.

[dustymabe]

Wasn't the idea behind those non exclusive test to be able to split to single ro test into multiple ones without a significant cost?

That was a huge motivator. We had a large file that had 20+ tests in it and we wanted to separate those into their own test files (with rationale and reasoning and git history and all) without requiring to run on 20+ VMs.

I agree that we want to reduce the cost of the tests as much as possible and at the same time I think we should be explicit when we want non RO tests to share a VM.

I'm not sure which side you are arguing for here. Are you arguing for a new flag that says "Yes, I'm not exclusive, but I need some RW access?"

[travier]

I'm not sure which side you are arguing for here. Are you arguing for a new flag that says "Yes, I'm not exclusive, but I need some RW access?"

I see good arguments on both sides 🙂