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.

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

Auto-detect WORKER_HOSTNAME on worker startup if not set explicitly #4900

Merged
merged 1 commit into from Nov 15, 2022

Conversation

Martchus
Copy link
Contributor

  • Make it unnecessary to specify WORKER_HOSTNAME in workers.ini if hostname --fqdn returns the expected name.
  • Tested the used functions of the Net::Domain module on multiple production worker hosts (using Leap 15.3 and 15.4). They behave as expected (so workers will still register using the short hostname and WORKER_HOSTNAME will be set to hostname --fqdn).
  • Required on worker level for the developer mode (independently of how the variable is handled by os-autoinst)
  • See https://progress.opensuse.org/issues/120261#note-8

* Make it unnecessary to specify `WORKER_HOSTNAME` in `workers.ini`
  if `hostname --fqdn` returns the expected name.
* Tested the used functions of the `Net::Domain` module on multiple
  production worker hosts (using Leap 15.3 and 15.4). They behave as
  expected (so workers will still register using the short hostname and
  `WORKER_HOSTNAME` will be set to `hostname --fqdn`).
* Required on worker level for the developer mode (independently of how the
  variable is handled by os-autoinst)
* See https://progress.opensuse.org/issues/120261#note-8
@codecov
Copy link

codecov bot commented Nov 15, 2022

Codecov Report

Merging #4900 (79ab25d) into master (31338e1) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4900   +/-   ##
=======================================
  Coverage   98.14%   98.14%           
=======================================
  Files         379      379           
  Lines       35233    35233           
=======================================
  Hits        34581    34581           
  Misses        652      652           
Impacted Files Coverage Δ
lib/OpenQA/Worker.pm 95.58% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

okurz added a commit to okurz/os-autoinst that referenced this pull request Nov 15, 2022
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

How about you run this for a day on w11/w12 connected to OSD for testing?

@Martchus
Copy link
Contributor Author

I can do that. Of course it will only be worthwhile when also removing WORKER_HOSTNAME from workers.ini so I'd have to do that manually (temporarily removing the workers from salt).

@Martchus
Copy link
Contributor Author

I have removed WORKER_HOSTNAME on both hosts and stopped salt. Then I have deployed the change on both workers:

zypper ar --priority 85 https://download.opensuse.org/repositories/devel:/openQA:/GitHub:/os-autoinst:/openQA:/PR-4900/openSUSE_Leap_15.4 pr-4900
zypper in pr-4900:openQA-worker

Judging by the worker logs it already looks good.

@mergify mergify bot merged commit 72a731b into os-autoinst:master Nov 15, 2022
@Martchus Martchus deleted the worker-hostname-dedection branch November 15, 2022 16:37
@perlpunk
Copy link
Contributor

os-autoinst-bot pushed a commit to os-autoinst-bot/openQA that referenced this pull request Nov 16, 2022
commit 72a731b
Merge: 8f456e6 79ab25d
Author:     mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
AuthorDate: Tue Nov 15 16:33:20 2022 +0000
Commit:     GitHub <noreply@github.com>
CommitDate: Tue Nov 15 16:33:20 2022 +0000

    Merge pull request os-autoinst#4900 from Martchus/worker-hostname-dedection

    Auto-detect `WORKER_HOSTNAME` on worker startup if not set explicitly
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.

None yet

3 participants