Skip to content

Adjustments for SUSE manager for retail - Rough network setting#386

Merged
Bischoff merged 2 commits intouyuni-project:masterfrom
Bischoff:retail-adjustments-3
Sep 5, 2018
Merged

Adjustments for SUSE manager for retail - Rough network setting#386
Bischoff merged 2 commits intouyuni-project:masterfrom
Bischoff:retail-adjustments-3

Conversation

@Bischoff
Copy link
Copy Markdown
Contributor

@Bischoff Bischoff commented Jul 5, 2018

This pull request adds support for SUSE Manager for retail:

    +------------+-----------+------------+  10.160.0.0/16
    |            |           |            |
+---+----+   +---+---+   +---+----+   +---+----+
|  eth0  |   |  eth0 |   |  eth0  |   |  eth0  |
|        |   |       |   |        |   |        |
| server |   | proxy |   | client |   | minion |
|        |   |       |   |        |   |        |
+  eth1  |   |  eth1 |   |  eth1  |   |  eth1  |
+---+----+   +---+---+   +---+----+   +---+----+
    |            |           |            |
    +------------+-----------+------------+  192.168.5.0/24
  • variable geometry bits:
    • new retail base setting
    • corresponding retail grain on the hosts
    • corresponding $RETAIL variable in the .bashrc on the host
  • minimalistic network setup if in retail mode:
    • additional private network
    • a second interface to that network on each host
  • hacks: needed repos and packages
    • retail "devel" repo (*)
    • branch network formula (**)
    • dhcp server and SuSEfirewall2 packages (**)

The plan is to rebase the finer setup from #383 on top of this PR.

(*) should go away when merged with suse manager
(**) should go away when we have minions as proxies

@Bischoff Bischoff requested a review from moio July 5, 2018 10:04
@Bischoff Bischoff changed the title Support for Retail: tests part Support for Retail: rough network setting Jul 5, 2018
@Bischoff Bischoff removed the request for review from moio July 5, 2018 11:36
@Bischoff Bischoff changed the title Support for Retail: rough network setting [WIP] Adjustments for SUSE manager for retail - Rough network setting Jul 5, 2018
@Bischoff Bischoff requested a review from moio July 9, 2018 07:39
@Bischoff Bischoff changed the title [WIP] Adjustments for SUSE manager for retail - Rough network setting Adjustments for SUSE manager for retail - Rough network setting Jul 9, 2018
@aaannz
Copy link
Copy Markdown
Contributor

aaannz commented Jul 24, 2018

One thing; retail as a solution can have two network topologies. One specified in PR (machine with dedicated network interface for internal network) and one where there is only one NIC.

Example:

WAN <----> [ROUTER] --+-----------+------------+------------+--
                      |           |            |            |
                   [BRANCH]  [TERMINAL1]  [TERMINAL2]  [TERMINAL3]  ...

That's why I am not so sure about naming this toggle retail. I would prefer something descriptive i.e. dedicated_network or internal_net or similar.

@aaannz
Copy link
Copy Markdown
Contributor

aaannz commented Jul 25, 2018

Still, even in "retail-only" module there are two options for network - one separate and one to leave it as is, but setup libvirt dhcp to use next-server and dns from branch.

The thing is that I thought for years that having dedicated internal network was how the retail solution was used. This kickoff I learned from Jeff that it is not the case and instead this shared network is used in majority.

@Bischoff
Copy link
Copy Markdown
Contributor Author

In the context of this PR, there are 3 things:
1 - "variable geometry" bits - needed in all the cases to make retail tests optional. It could be controlled by a retail flag.
2 - a special network setup, that I need if I want to test in place the branch network, DHCP, and DNS formulas. It could be controlled by a second flag, for example branch_network.
3 - hacks, that are temporary only ; some will go away when Retail team's work is merged, and the others will go when we have the "proxy as minion" functionality. For the sake of simplicity, these hacks should also be controlled by retail flag.

As Ondrej pointed out, Retail can be tested in a second network setup, where the formulas are not used because DHCP and DNS are on a separated box (the so-called "router"). This is not covered in this PR, but if it is needed, I could add it without too much effort. I'm not sure though this would bring much to the testing side of things...

To sum up the needed flags:

  • retail : variable geometry bits, temporary hacks
  • branch_network : a special network including the clients and the branch server
  • a third flag that would control whether the branch server is also a router / dhcp / dns box or whether those functionalities are external.

I have to check, but the retail flag is probably not needed in base module, while the other one probably needs to be there.

Does that make sense to you, @aaannz ? @moio ?

@moio
Copy link
Copy Markdown
Contributor

moio commented Aug 28, 2018

I suggest renaming the retail variable in base as additional_network and its output variable additional_network_id.

It would be cool if it also had additional_network_name/additional_network_bridge/additional_network_mac to have the same expressing power of the primary network, but this is only "nice to have" - I am fine with anything that makes testing retail scenarios possible.

What will be needed is OpenStack support as this is not a libvirt exclusive. I can offer to help testing.

Thank you very much

@MalloZup MalloZup self-requested a review September 3, 2018 16:42
Copy link
Copy Markdown
Contributor

@MalloZup MalloZup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@Bischoff
Copy link
Copy Markdown
Contributor Author

Bischoff commented Sep 3, 2018

Current status: I have divided retail variable into 3 variables into 3 different modules:

  • private_network (base): use a private network?
  • retail_repo (server): install Retail team repository and packages? temporary
  • retail (controller): should we run SUSE Manager for Retail tests?

retail_network_id was renamed to private_network_id.

What is missing:

  • support in cloud
  • possibility to have the private network not configured via formulas

@MalloZup
Copy link
Copy Markdown
Contributor

MalloZup commented Sep 4, 2018

@Bischoff for retail_repo i would not create a variable but use the built-in:
https://github.com/moio/sumaform/blob/master/README_ADVANCED.md#add-custom-repos-and-packages

(this is more flexible then adding new temp variable)

for :

  • retail (controller): should we run SUSE Manager for Retail tests?

My opinion is that we can have a mechanism for disabling Retail tests but imho we should run them together with the HEAD testsuite.

If we have 2 differents testsuite to me is hardware/resource waste and also people are struggling revieing 1 head, imagine to have 2 differents stuff. ( this is the same pattern we had seen with SLE15 tests, which went not well)

For the rest ok

@Bischoff
Copy link
Copy Markdown
Contributor Author

Bischoff commented Sep 4, 2018

@Bischoff for retail_repo i would not create a variable but use the built-in:

yes, good idea.

retail (controller): should we run SUSE Manager for Retail tests?
My opinion is that we can have a mechanism for disabling Retail tests but imho we should run them together
with the HEAD testsuite.

It was the plan....

Well, not immediately in HEAD, but in 3.2, and once it works in 3.2, in HEAD too. Why so? Because Retail is a 3.2 MU in the first place.

If we have 2 differents testsuite to me is hardware/resource waste

I never wanted to do that 😋 .

Copy link
Copy Markdown
Contributor

@moio moio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to merge, small fixups might come later.

Thanks for all the work here

pool = "${var.pool}"
}

resource "libvirt_network" "private_network" {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to name this resource additional_network for consistency?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will do.

@uyuni-project uyuni-project deleted a comment from MalloZup Sep 5, 2018
@uyuni-project uyuni-project deleted a comment from aaannz Sep 5, 2018
@uyuni-project uyuni-project deleted a comment from moio Sep 5, 2018
@uyuni-project uyuni-project deleted a comment from MalloZup Sep 5, 2018
@uyuni-project uyuni-project deleted a comment from MalloZup Sep 5, 2018
@uyuni-project uyuni-project deleted a comment from moio Sep 5, 2018
@uyuni-project uyuni-project deleted a comment from moio Sep 5, 2018
@uyuni-project uyuni-project deleted a comment from moio Sep 5, 2018
@Bischoff
Copy link
Copy Markdown
Contributor Author

Bischoff commented Sep 5, 2018

Ok, I'm convinced by @MalloZup, let's remove retail variable completly and have only one test suite.
However, I will need not to merge the new testsuite features now as they would be too red. But waiting is enough, we don't need a flag.

To answer @moio's comment, for the moment there is no additional_network_name : the name is currently derived from prefix and private, eg ebi3-private, we could change that. The additional network in Retail context cannot be bridged to the outside. I don't see well why we would need to specify a MAC.

If or when we add the possibility to externally set the additional network, we will have to be creative for a new variable.

@Bischoff
Copy link
Copy Markdown
Contributor Author

Bischoff commented Sep 5, 2018

@mbologna the retail repos are installed on the minions too because of Kiwi packages. What about moving this hack out of sumaform as well and into the main.tf files with @MalloZup's trick?

(since I have everything ready, I can do it for you).

@mbologna
Copy link
Copy Markdown
Contributor

mbologna commented Sep 5, 2018

@mbologna the retail repos are installed on the minions too because of Kiwi packages. What about moving this hack out of sumaform as well and into the main.tf files with @MalloZup's trick?

(since I have everything ready, I can do it for you).

I agree with moving this HACK into main.tf but please remember that it's temporary and it will be removed as soon as the FATE entry linked will be closed.
Thanks for taking care of that

@Bischoff
Copy link
Copy Markdown
Contributor Author

Bischoff commented Sep 5, 2018

I agree with moving this HACK into main.tf but please remember that it's temporary
and it will be removed as soon as the FATE entry linked will be closed.

Of course. My hack too is temporary.

Thanks for taking care of that

No worries, while I'm at it...

@mbologna
Copy link
Copy Markdown
Contributor

mbologna commented Sep 5, 2018

My advice is: if you don't need to invest too much time in it && you like more having this HACK in main.tf, then yes, go ahead.

@MalloZup
Copy link
Copy Markdown
Contributor

MalloZup commented Sep 5, 2018

i have seen temporary hack for 10 years 👍 🤣

@Bischoff
Copy link
Copy Markdown
Contributor Author

Bischoff commented Sep 5, 2018

Temporary hack removed, merging.

@Bischoff Bischoff merged commit 0d7fd02 into uyuni-project:master Sep 5, 2018
@Bischoff Bischoff deleted the retail-adjustments-3 branch September 5, 2018 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants