Both branches are the same, is this intentional? Looks correct, but then an OR instead of a separate if may be in order.

It was supposed to keep the line short(er), using those names is a bit ridiculous. I'll change to an ||.

Oh, someone actually reads those? =p

A fluke :D

I know this only follows what was done for the first template, but I only noticed now. If we intend ci-operator files to be usable (readable/writable) by humans, we should avoid non-intuitive clutter like this, even for a cost of slightly more complicated implementation. How hard would it be to make it:

- as: ...
  openshift_installer_src:
    cluster_profile: random

?

It's probaby not a matter of this PR, but I think we're still early enough in the IaaS-agnostic job work to make that change.

That makes sense, I didn't really think about it too long before deciding on the name. How about omitting the cluster profile altogether? I don't really like random and I think "not tied to a specific target" is a good default. This way we can remove random from the job type as well.

I liked what you proposed at first (even wrote a comment in that direction), but after more thinking I think we should keep the explicit config. I think the agnostic behavior could make a good default, but its not intuitive to whoever is trying to read and write these configs. We know every bit and quirk of it, but that's not the case for everyone.

(to be entirely clear, I did not originally like the {} part of the config)

Some API Objects have a type field for this purpose when the type-specific config does not exist or is optional

Ignoring the problem of how to structure the configuration (more on that below) I think random being the default value makes the configuration more intuitive, not less.

If the test needs a cluster, use openshift_installer/openshift_installer_src. If it needs a cluster with a specific configuration, add cluster_profile: …. The former should be the default choice for test authors if the test templates do their job and all supported targets are virtually equivalent. The details of what cluster is created should not concern them.

As for the configuration, we have these test types (ignoring ansible, console, and the two random types I am proposing we remove):

• container
• openshift_installer
• openshift_installer_src
• openshift_installer_upi

container tests must have a from field. openshift_installer* tests must have a cluster_type.

I don't particularly like the empty dictionary, but there are examples even in Kubernetes (e.g.). Adding an extra field to all types seems silly in this case: unless we default the value if one of the pointers is set, which doesn't seem very ellegant either, we would end up with:

  type: container
  container:
    # …

Additionally, if we ever add a field to the cluster types (openshift_installer has an optional upgrade field), that becomes entirely redundant.

I think random being the default value makes the configuration more intuitive, not less.

I'm trying to imagine this from the PoV of a generic OpenShift engineer who does not know ci-operator configs very well. Currently the config is telling them quite well what they have set up, "smart" defaults defeat that.

The details of what cluster is created should not concern them: This IMHO makes sense only from high-level, not from individual's PoV.

Not really, the point of k8s and openshift in general is to be portable cross-cloud and not worry at all, so the default idea that it matters not which infra is underneath is a good one.

Good discussion. I'm not entirely convinced, but I do not need to be. I'm OK with making it that way.