sumaform icon indicating copy to clipboard operation
sumaform copied to clipboard

Have the test suite module work with several clients, minions, SSH minions, etc.

Open Bischoff opened this issue 5 years ago • 6 comments

SUSE/spacewalk#10730

problem 1

Identifiers for the different client types lead to absurdities. Instead of

    cli-sles12sp4 = {
      name = "client"
      image = "opensuse152o"
    }

we should have something like:

    suse_client = {
      name = "client"
      image = "opensuse152o"
    }

problem 2

Testsuite module accepts 0 or 1 of each client, it should be 0 to N, especially for the QAM test suite:

  suse_clients = [
       client1 = {
          name = "client-leap-15.2"
          image = "opensuse152o"
    }, client2 = {
          name = "client-sles-15.1"
          image = "sles15sp1"
    }
   ]

problem 3

For big setups like the QAM test suite, and for the uyuni test suite, we have to be able to specify different providers in base module. It could be several libvirt hosts, or even a mix of libvirt and cloud hosts.

This is currently possible when not using the cucumber test suite module, with things like that:

module "cli-sles15sp1" {
  providers = {
    libvirt = libvirt.classic181
  }
  source             = "./modules/client"
  base_configuration = module.base3.configuration
  ...
}

module "base3" {
  providers = {
    libvirt = libvirt.classic181
  }
  source = "./modules/base"
  provider_settings = {
    pool        = "default"
    bridge      = "br0"
    additional_network = "192.168.40.0/24"
  }
  ...
}

I'm not sure if we can achieve this with current test suite module.

Bischoff avatar Jul 17 '20 11:07 Bischoff

I see problems 1 and 2 as related, and strictly dependent on the testsuite code: it was structured the way it is to follow what the testsuite needed. If the testsuite becomes more flexible in accepting VMs, my opinion is that we should follow that flexibility in the same way in sumaform accordingly.

Question is: does the Cucumber testsuite accept only one "SUSE client" and the version is a variable, or should it accept an array of clients with a bunch of OS's? I tend to think the latter is preferable. So one idea could be to expect an array of clients, each of which will have an OS and some flags or a label that indicates its "role" (traditional client, minion, image build host...).

About problem 3, there is the possibility of defining multiple providers in the top-level main.tf file and then pass a reference to a certain provider down in the modules. It requires some surgery but it can be done, and I do not think there's much of an alternative in this case (unless we want multiple separated main.tfs, which is probably worse).

Thanks for bringing this up

moio avatar Jul 20 '20 07:07 moio

it was structured the way it is to follow what the testsuite needed.

Well, no. It was never "needed" that the test suite handled "SLE 12 SP4 minion". It was never "needed" that the test suite conformed to our (censored) naming of VMs. What was needed was always that the test suite handled a "SUSE family minion" as something generic.

This is mostly an effect of miscommunication. I am probably to blame, for not having detected the issue when I was requested to have a look at the PRs. Sorry, my bad.

Question is: does the Cucumber testsuite accept only one "SUSE client" and the version is a variable, or should it accept an array of clients with a bunch of OS's? I tend to think the latter is preferable.

The current test suite accepts only one client, but the upcoming QAM test suite will need several of them. Currently we work around this by not using the test suite module:

https://github.com/srbarrios/susemanager-ci/blob/migrate-qam-env/terracumber_config/tf_files/SUSEManager-4.0-qam.tf

Apart from this new testsuite, I often feel the need for my own private tests to have several machines of one kind.

So one idea could be to expect an array of clients, each of which will have an OS and some flags or a label that indicates its "role" (traditional client, minion, image build host...).

That is indeed Ricardo's idea and I think that's the way to go.

A possible alternative could be to plan for, say, a maximum 8 clients and hardcode them:

   suse-family-client = {
          name = "client-leap-15.2"
          image = "opensuse152o"
    }
    suse-family-client-2 = {
          name = "client-sles-15.1"
          image = "sles15sp1"
    }

That would avoid the extra burden of declaring an array. @rjmateus, what do you think, would that be doable?

About problem 3, there is the possibility of defining multiple providers in the top-level main.tf file and then pass a reference to a certain provider down in the modules. It requires some surgery but it can be done, and I do not think there's much of an alternative in this case (unless we want multiple separated main.tfs, which is probably worse).

The idea is precisely to avoid several main.tfs. Again, see

https://github.com/srbarrios/susemanager-ci/blob/migrate-qam-env/terracumber_config/tf_files/SUSEManager-4.0-qam.tf

Apart from the new QAM test suite, this might also be needed for running the uyuni test suite because it might be half local, half in the cloud.

Thanks for bringing this up

Sure. I will take care of it.

Bischoff avatar Jul 20 '20 08:07 Bischoff

The idea was to accept several clients with a bunch of OS'es.

However, this proved impossible with HCL: for_each does not work in modules.

So my PR will solve problem 1 only. For problem 2 and 3, we'll have to wait until terraform includes a real programming language.

Bischoff avatar Jul 22 '20 17:07 Bischoff

However, this proved impossible with HCL: for_each does not work in modules.

https://github.com/hashicorp/terraform/tree/guide-v0.13-beta/module-repetition#terraform-v013-beta-module-for_each-and-count

Will that help?

moio avatar Jul 24 '20 07:07 moio

Yes Moio, that is exactly what is needed to implement this card. Eric didn't close this card so we can wait for the feature to be available and then revisit it on that time

rjmateus avatar Jul 24 '20 08:07 rjmateus

Yes. For now I just address the renaming (problem 1).

Bischoff avatar Jul 27 '20 09:07 Bischoff