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

MGMT-16843: Use hostnamectl to replace illegal hostname #839

Merged
merged 1 commit into from
May 23, 2024

Conversation

CrystalChun
Copy link
Contributor

@CrystalChun CrystalChun commented Apr 24, 2024

Hosts may not restart before beginning installation
so if we generate a random hostname, it won't take
effect unless we use hostnamectl to actually replace
the hostname live.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Apr 24, 2024

@CrystalChun: This pull request references MGMT-16843 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.16.0" version, but no target version was set.

In response to this:

SNO clusters don't restart the hosts before beginning installation so if we generate a random hostname and write it to /etc/hostname, it won't take effect unless the hostname service is restarted.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 24, 2024
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 24, 2024
Copy link

openshift-ci bot commented Apr 24, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 24, 2024
Copy link

openshift-ci bot commented Apr 24, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CrystalChun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 24, 2024
@CrystalChun
Copy link
Contributor Author

/test ?

Copy link

openshift-ci bot commented Apr 24, 2024

@CrystalChun: The following commands are available to trigger required jobs:

  • /test e2e-agent-compact-ipv4
  • /test edge-e2e-ai-operator-ztp
  • /test edge-e2e-metal-assisted
  • /test edge-format-check
  • /test edge-images
  • /test edge-lint
  • /test edge-unit-test
  • /test images
  • /test mce-images

The following commands are available to trigger optional jobs:

  • /test e2e-agent-ha-dualstack
  • /test e2e-agent-sno-ipv6
  • /test edge-e2e-metal-assisted-cnv
  • /test edge-e2e-metal-assisted-external
  • /test edge-e2e-metal-assisted-ipv6
  • /test edge-e2e-metal-assisted-lvm
  • /test edge-e2e-metal-assisted-odf
  • /test edge-e2e-metal-assisted-single-node
  • /test edge-e2e-oci-assisted
  • /test edge-e2e-oci-assisted-4-14
  • /test edge-e2e-vsphere-assisted
  • /test edge-e2e-vsphere-assisted-4-13
  • /test edge-e2e-vsphere-assisted-4-14
  • /test edge-publish-multi-arch-images-dry-run

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-openshift-assisted-installer-master-e2e-agent-compact-ipv4
  • pull-ci-openshift-assisted-installer-master-edge-e2e-ai-operator-ztp
  • pull-ci-openshift-assisted-installer-master-edge-e2e-metal-assisted
  • pull-ci-openshift-assisted-installer-master-edge-format-check
  • pull-ci-openshift-assisted-installer-master-edge-images
  • pull-ci-openshift-assisted-installer-master-edge-lint
  • pull-ci-openshift-assisted-installer-master-edge-unit-test
  • pull-ci-openshift-assisted-installer-master-images
  • pull-ci-openshift-assisted-installer-master-mce-images

In response to this:

/test ?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@CrystalChun
Copy link
Contributor Author

/test e2e-agent-sno-ipv6 e2e-agent-compact-ipv4


if highAvailabilityMode == models.ClusterHighAvailabilityModeNone {
i.log.Infof("Restarting systemd-hostnamed service for SNO after writing new hostname [%s]", data)
i.ops.SystemctlAction("restart", "systemd-hostnamed")
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering, maybe it is just easier to run hostnamectl command for all cases? it should set everything as expected without need to restart service?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea to use hostnamectl, will update. Do you also mean to do it on multi-node clusters too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested both ways and I get this when trying to access the installed spoke cluster

Unable to connect to the server: tls: failed to verify certificate: x509: certificate signed by unknown authority (possibly because of "crypto/rsa: verification error" while trying to verify candidate authority certificate "kube-apiserver-lb-signer")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I send the requested hostname to the installer?

Copy link
Contributor

Choose a reason for hiding this comment

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

very strange, hostname is not part of kubeconfig

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah sorry this error was a complete fluke. Something weird with my env 😅 Made the change to use hostnamectl to set the hostname for any cluster (not just SNO)!

Hosts may not restart before beginning installation
so if we generate a random hostname, it won't take
effect unless we use hostnamectl to actually replace
the hostname live.
@CrystalChun
Copy link
Contributor Author

/test edge-unit-test edge-e2e-metal-assisted edge-e2e-metal-assisted-single-node

@CrystalChun
Copy link
Contributor Author

CrystalChun commented May 15, 2024

/retitle MGMT-16843: Use hostnamectl to replace illegal hostname

@openshift-ci openshift-ci bot changed the title MGMT-16843: Restart hostnamed service after replacing hostname MGMT-16843: Use hostnamectl to replace illegal hostname May 15, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented May 15, 2024

@CrystalChun: This pull request references MGMT-16843 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.16.0" version, but no target version was set.

In response to this:

Hosts may not restart before beginning installation
so if we generate a random hostname, it won't take
effect unless we use hostnamectl to actually replace
the hostname live.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Copy link

codecov bot commented May 15, 2024

Codecov Report

Attention: Patch coverage is 28.57143% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 55.68%. Comparing base (f548d32) to head (651ca29).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #839      +/-   ##
==========================================
- Coverage   55.73%   55.68%   -0.06%     
==========================================
  Files          15       15              
  Lines        3199     3202       +3     
==========================================
  Hits         1783     1783              
- Misses       1245     1247       +2     
- Partials      171      172       +1     
Files Coverage Δ
src/ops/ops.go 43.30% <0.00%> (ø)
src/installer/installer.go 69.25% <40.00%> (-0.37%) ⬇️

@CrystalChun
Copy link
Contributor Author

/test edge-e2e-ai-operator-ztp

Forgot ci/prow/edge-e2e-metal-assisted is actually broken https://issues.redhat.com/browse/OCPBUGS-33493

@tsorya
Copy link
Contributor

tsorya commented May 15, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 15, 2024
@CrystalChun CrystalChun marked this pull request as ready for review May 15, 2024 15:59
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 15, 2024
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD f548d32 and 2 for PR HEAD 651ca29 in total

@tsorya
Copy link
Contributor

tsorya commented May 19, 2024

/retest

1 similar comment
@CrystalChun
Copy link
Contributor Author

/retest

@CrystalChun
Copy link
Contributor Author

/retest-required

Copy link

openshift-ci bot commented May 23, 2024

@CrystalChun: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 9e2f22a into openshift:master May 23, 2024
11 checks passed
@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-agent-installer-orchestrator-container-v4.17.0-202405230209.p0.g9e2f22a.assembly.stream.el9 for distgit ose-agent-installer-orchestrator.
All builds following this will include this PR.

@CrystalChun
Copy link
Contributor Author

/cherry-pick release-ocm-2.10 release-ocm-2.9 release-4.16 release-4.15

@openshift-cherrypick-robot

@CrystalChun: new pull request created: #848

In response to this:

/cherry-pick release-ocm-2.10 release-ocm-2.9 release-4.16 release-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@CrystalChun
Copy link
Contributor Author

/cherrypick release-ocm-2.9 release-4.16 release-4.15

@openshift-cherrypick-robot

@CrystalChun: new pull request created: #849

In response to this:

/cherrypick release-ocm-2.9 release-4.16 release-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@CrystalChun
Copy link
Contributor Author

/cherrypick release-4.16 release-4.15

@openshift-cherrypick-robot

@CrystalChun: new pull request created: #850

In response to this:

/cherrypick release-4.16 release-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants