Happy-path tests should use values in che.properties when running tests

[amisevsk]

Is your enhancement related to a problem? Please describe.

Currently, happy path tests use the Che operator to deploy Che and then run tests. This can cause issues when testing a PR that updates a Che property that defines an image, since the Operator will cause tests to be run with the outdated property (e.g. #17785, which updates brokers and fails to run workspaces if older brokers are used).

Describe the solution you'd like

If possible, the happy path tests should make sure no properties in che.properties are overridden when running tests, making che.properties the primary source of truth for default configuration. Otherwise, any changes that attempt to update e.g. a default image and change functionality will fail to pass happy-path.

Describe alternatives you've considered

The workaround for this is to merge property changes in a separate PR, wait for the changes to be available in the operator, and then open a new PR with the rest of the changes required. However, this is only possible when the changes are independent -- if we wanted to change the broker in a way that required Che server changes, it would be impossible.

Additional context

PRs #17785, #17870

[sparkoo]

I don't think it makes much sense for few reasons:

1. they're e2e test. If we rule out the che-operator from the chain, we're not testing the installation part, which is imho big part. In real environment, che.properties are almost never the highest priority and happy path tests follow that.
2. I don't think it's even possible with operator. You would need to unset all env variables that che-operator creates in che configmap. So this setup would be very crooked, which again would not test the real environment.
3. I don't think we should tell how to run these e2e tests and rather take them as blackbox, so we're not developing only to pass the tests. (that's for another discussion though. It probably does not apply here.)

I think solution for this might be something like provide custom che-operator image or CheCluster patch needed to run the tests. But again, this will expose the "implementation details" of the testsuite.

I'd like to know QE opinion on this. @rhopp ? For now, setting it as info-needed so we can discuss what's the right thing to do here.

[amisevsk]

@sparkoo Well, the issue is that we can't update just the Che operator, since its config settings are derived from che.properties, and we may not be able to update che.properties without merging a PR that fails happy path.

For example, if we wanted to add a mandatory config value to the broker, without a default, we'd run into a chicken-and-egg problem: we can't update the Che operator to use the new version of the broker because that's not what's in che.properties, and we can't update the Che server + properties because the operator will override the change. The current happy path tests test partially-applied fixes.

[che-bot]

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

[nallikaea]

Outdated