Correct hostname retrieval from Source::getHostname #38309

4quarks · 2022-06-09T16:15:59Z

PR description:

The piece of code [1] does the following: get the position of the first colon ":" and defines the hostname variable with anything that is before that colon. This supposition is based on the case that you have a hostname and a port i.e. cms-xrd-global01.cern.ch:1094 --> hostname is cms-xrd-global01.cern.ch. However, the information is not always available and therefore, when a raw IPv6 gets there, the output is incorrect as the information before the first colon is just the first octet of the IP i.e.[2001 from [2001:760:4205:128::128:232]:1094.

With the small change of using the cpp function find_last_of this problem is solved. Because it will try to find the last colon and get everything before it. This resolves both scenarios cms-xrd-global01.cern.ch:1094 and [2001:760:4205:128::128:232]:1094.

Thanks in advance.

[1]

bool Source::getHostname(const std::string &id, std::string &hostname) {
  size_t pos = id.find_last_of(':');
  hostname = id;
  if ((pos != std::string::npos) && (pos > 0)) {
    hostname = id.substr(0, pos);

PR validation:

You can run a manual HammerCloud test as explained on the Twiki https://twiki.cern.ch/twiki/bin/view/CMSPublic/CMSHammerCloud#Manual_execution
I've already done it and it works fine.

if this PR is a backport please specify the original PR and why you need to backport that PR:

This could be backported even to the CMSSW_9_2_6 version which is what is being used in production on HammerCloud.

cmsbuild · 2022-06-09T16:21:42Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38309/30473

This PR adds an extra 16KB to repository

cmsbuild · 2022-06-09T16:22:02Z

A new Pull Request was created by @4quarks (4Quarks) for master.

It involves the following packages:

Utilities/XrdAdaptor (core)

@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks.
@wddgit this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

cmsbuild · 2022-06-09T16:47:34Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38309/30474

This PR adds an extra 16KB to repository

cmsbuild · 2022-06-09T16:47:52Z

Pull request #38309 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again.

qliphy · 2022-06-29T01:06:01Z

please test

cmsbuild · 2022-06-29T05:17:14Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3d8148/25879/summary.html
COMMIT: 95c05b9
CMSSW: CMSSW_12_5_X_2022-06-28-1500/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38309/25879/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3654586
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3654562
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 208 log files, 45 edm output root files, 50 DQM output files
TriggerResults: no differences found

Dr15Jones · 2022-06-29T18:41:10Z

Utilities/XrdAdaptor/src/XrdSource.cc

  hostname = id;
  if ((pos != std::string::npos) && (pos > 0)) {
    hostname = id.substr(0, pos);
+    hostname.erase(remove(hostname.begin(), hostname.end(), ']'), hostname.end());
+    hostname.erase(remove(hostname.begin(), hostname.end(), '['), hostname.end());


So your intent is to change something like [2001:760:4205:128::128:232]:1094 to be 2001:760:4205:128::128:232 ?

Would it make sense to restrict the '[' to be at the beginning of the string and ']' to be at the end in order to remove it?

It looks like the commit slipped in the PR. You can skip those changes. Thanks.

@4quarks Could you drop the commit then? (that is pretty much the only way for us to "skip changes")

cmsbuild · 2022-07-15T08:01:24Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38309/31064

This PR adds an extra 16KB to repository

cmsbuild · 2022-07-15T08:01:45Z

Pull request #38309 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again.

makortel · 2022-07-15T15:17:11Z

@cmsbuild, please test

cmsbuild · 2022-07-15T19:15:50Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3d8148/26267/summary.html
COMMIT: 213b684
CMSSW: CMSSW_12_5_X_2022-07-15-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38309/26267/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3654053
DQMHistoTests: Total failures: 13
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3654017
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 49 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 208 log files, 45 edm output root files, 50 DQM output files
TriggerResults: no differences found

makortel · 2022-07-15T19:21:37Z

+1

cmsbuild · 2022-07-15T19:21:58Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

makortel · 2022-07-15T19:22:20Z

if this PR is a backport please specify the original PR and why you need to backport that PR:

This could be backported even to the CMSSW_9_2_6 version which is what is being used in production on HammerCloud.

@4quarks Are you planning to take care of the backports?

qliphy · 2022-07-16T01:04:45Z

+1

Correct hostname retrieval from Source::getHostname

4a37491

cmsbuild added this to the CMSSW_12_5_X milestone Jun 9, 2022

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures tests-pending labels Jun 9, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 9, 2022

Correcting the IPv6 DNS resolution

95c05b9

cmsbuild added code-checks-pending and removed code-checks-approved labels Jun 9, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 9, 2022

cmsbuild added tests-started and removed tests-pending labels Jun 29, 2022

cmsbuild added tests-approved and removed tests-started labels Jun 29, 2022

Dr15Jones reviewed Jun 29, 2022

View reviewed changes

revert ipv6 brackets removal

f68c36d

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Jul 15, 2022

Merge branch 'cms-sw:master' into hostname_correct

213b684

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 15, 2022

cmsbuild added tests-started and removed tests-pending labels Jul 15, 2022

cmsbuild added tests-approved and removed tests-started labels Jul 15, 2022

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Jul 15, 2022

cmsbuild added orp-approved and removed orp-pending labels Jul 16, 2022

cmsbuild merged commit 035f882 into cms-sw:master Jul 16, 2022

makortel mentioned this pull request May 11, 2023

[10_6_X] Fixes for XRootD 5.x #41599

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correct hostname retrieval from Source::getHostname #38309

Correct hostname retrieval from Source::getHostname #38309

4quarks commented Jun 9, 2022

cmsbuild commented Jun 9, 2022

cmsbuild commented Jun 9, 2022

cmsbuild commented Jun 9, 2022

cmsbuild commented Jun 9, 2022

qliphy commented Jun 29, 2022

cmsbuild commented Jun 29, 2022

Dr15Jones Jun 29, 2022

4quarks Jul 14, 2022

makortel Jul 14, 2022

cmsbuild commented Jul 15, 2022

cmsbuild commented Jul 15, 2022

makortel commented Jul 15, 2022

cmsbuild commented Jul 15, 2022

makortel commented Jul 15, 2022

cmsbuild commented Jul 15, 2022

makortel commented Jul 15, 2022

if this PR is a backport please specify the original PR and why you need to backport that PR:

qliphy commented Jul 16, 2022

Correct hostname retrieval from Source::getHostname #38309

Correct hostname retrieval from Source::getHostname #38309

Conversation

4quarks commented Jun 9, 2022

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Jun 9, 2022

cmsbuild commented Jun 9, 2022

cmsbuild commented Jun 9, 2022

cmsbuild commented Jun 9, 2022

qliphy commented Jun 29, 2022

cmsbuild commented Jun 29, 2022

Comparison Summary

Dr15Jones Jun 29, 2022

Choose a reason for hiding this comment

4quarks Jul 14, 2022

Choose a reason for hiding this comment

makortel Jul 14, 2022

Choose a reason for hiding this comment

cmsbuild commented Jul 15, 2022

cmsbuild commented Jul 15, 2022

makortel commented Jul 15, 2022

cmsbuild commented Jul 15, 2022

Comparison Summary

makortel commented Jul 15, 2022

cmsbuild commented Jul 15, 2022

makortel commented Jul 15, 2022

if this PR is a backport please specify the original PR and why you need to backport that PR:

qliphy commented Jul 16, 2022