Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make server_configuration optionnal. #1736

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
+16 −6
Open

Make server_configuration optionnal. #1736

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3ea761e
Remove variables not impacting the deployment.
maximenoel8 Oct 9, 2024
bdbd539
Impact cucumber testsuite with last changes
maximenoel8 Oct 10, 2024
dab03d2
Add default username and password.
maximenoel8 Oct 10, 2024
738f4f4
Add default value to proxy
maximenoel8 Oct 10, 2024
b6e2440
Remove avahi_reflector from build host in cucumber module.
maximenoel8 Oct 10, 2024
5a92526
Fix hostname default variable.
maximenoel8 Oct 10, 2024
a916fe7
Revert variable changes in build_host
maximenoel8 Oct 10, 2024
7930779
Revert variable changes in build_host
maximenoel8 Oct 10, 2024
f3748e0
Add default value to server_configuration
maximenoel8 Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions modules/build_host/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ module "build_host" {
disable_firewall = var.disable_firewall
grains = {
mirror = var.base_configuration["mirror"]
server = var.server_configuration["hostname"]
auto_connect_to_master = var.auto_connect_to_master
Copy link
Member

@srbarrios srbarrios Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

From the point of view of auto_connect_to_master feature, I think it make sense to have the possibility to auto connect a build host, at the end is just another salt minion that we onboard with a special Add-On system type.

Instead of removing it because is unused at that moment, I would consider to change its parent from host to minion. So that would make possible for a build_host to auto-connect to master.

If you agree on that, we can create a card to track it and don't forget.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just that it has never been used for a long while ( maybe ever ). I was thinking adding the auto connect to build host, but if it's not needed now, I don't see a need for later.
I will ping Pablo to have a look at this PR.

Copy link
Contributor

@Bischoff Bischoff Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I used to use avahi quite a lot. These days a lot less, because of our private DNS entries, but it's a privilege. I know at least 2 developers who rely on avahi.

We don't need auto connect on the build host (*), but we do need the avahi reflector.

(*) at least not me, but maybe some developer or supporter does. Oscar mentioned that in demoes our supporters use that auto connect thing intensively.

Copy link
Member

Choose a reason for hiding this comment

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

I would agree with Óscar on moving the build_host from host to minion and then keeping the auto-connect with master feature.

As developer, I use avahi on my deployments all the times, OTOH I never use this reflector config, which seems not be actually working at the moment for build_host (as only does stuff on minion SLS files)

avahi_reflector = var.avahi_reflector
sles_registration_code = var.sles_registration_code
}

Expand Down
14 changes: 0 additions & 14 deletions modules/build_host/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,11 @@ variable "product_version" {
default = null
}

variable "server_configuration" {
description = "use module.<SERVER_NAME>.configuration, see the main.tf example file"
}

variable "auto_connect_to_master" {
description = "whether this minion should automatically connect to the Salt Master upon deployment"
default = true
}

variable "use_os_released_updates" {
description = "Apply all updates from SUSE Linux Enterprise repos"
default = false
}

variable "avahi_reflector" {
description = "if using avahi, let the daemon be a reflector"
default = false
}

variable "disable_firewall" {
description = "whether to disable the built-in firewall, opening up all ports"
default = true
Expand Down
3 changes: 3 additions & 0 deletions modules/client/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ variable "product_version" {

variable "server_configuration" {
description = "use module.<SERVER_NAME>.configuration, see the main.tf example file"
default = {
hostname = ""
}
}

variable "auto_register" {
Expand Down
4 changes: 0 additions & 4 deletions modules/cucumber_testsuite/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,12 +383,8 @@ module "build-host" {
image = lookup(local.images, "build-host", "sles15sp4o")
name = lookup(local.names, "build-host", "min-build")

server_configuration = local.minimal_configuration

auto_connect_to_master = false
use_os_released_updates = true
ssh_key_path = "./salt/controller/id_rsa.pub"
avahi_reflector = var.avahi_reflector
install_salt_bundle = lookup(local.install_salt_bundle, "build-host", false)

additional_repos = lookup(local.additional_repos, "build-host", {})
Expand Down
5 changes: 5 additions & 0 deletions modules/proxy/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ variable "product_version" {

variable "server_configuration" {
description = "use module.<SERVER_NAME>.configuration, see README_ADVANCED.md"
default = {
hostname = ""
username = "admin"
password = "admin"
}
}

variable "disable_firewall" {
Expand Down
5 changes: 5 additions & 0 deletions modules/proxy_containerized/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ variable "helm_chart_url" {

variable "server_configuration" {
description = "use module.<SERVER_NAME>.configuration, see README_ADVANCED.md"
default = {
hostname = ""
username = "admin"
password = "admin"
}
}

variable "auto_configure" {
Expand Down
1 change: 0 additions & 1 deletion modules/sshminion/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ module "sshminion" {
disable_firewall = var.disable_firewall
grains = {
mirror = var.base_configuration["mirror"]
auto_connect_to_master = var.auto_connect_to_master
sles_registration_code = var.sles_registration_code
}

Expand Down
5 changes: 0 additions & 5 deletions modules/sshminion/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ variable "product_version" {
default = null
}

variable "auto_connect_to_master" {
description = "whether this minion should automatically connect to the Salt Master upon deployment"
default = false
}

variable "use_os_released_updates" {
description = "Apply all updates from SUSE Linux Enterprise repos"
default = false
Expand Down
Loading