OCPBUGS-44882: Add NTP sources to generated install-config.yaml

[jhernand]

In OpenShift 4.18 a new additionalNTPServers field was added to the baremetal platform section of the install-config.yaml file . This needs to be populated with the NTP sources provided by the user in order to correctly set time before installation of new nodes using the baremetal provisioning flow.

List all the issues related to this PR

https://issues.redhat.com/browse/OCPBUGS-44882

• New Feature
• Enhancement
• Bug fix
• Tests
• Documentation
• CI/CD

What environments does this code impact?

• Automation (CI, tools, etc)
• Cloud
• Operator Managed Deployments
• None

How was this code tested?

• assisted-test-infra environment
• dev-scripts environment
• Reviewer's test appreciated
• Waiting for CI to do a full test run
• Manual (Elaborate on how it was tested)
• No tests needed

Tested creating a baremetal cluster and checking the generated installer configuration file.

Checklist

• Title and description added to both, commit and PR.
• Relevant issues have been associated (see CONTRIBUTING guide)
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

[openshift-ci-robot]

@jhernand: This pull request references Jira Issue OCPBUGS-44882, which is invalid:

• expected the bug to target the "4.19.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

In OpenShift 4.18 a new additionalNTPServers field was added to the baremetal platform section of the install-config.yaml file . This needs to be populated with the NTP sources provided by the user in order to correctly set time before installation of new nodes using the baremetal provisioning flow.

List all the issues related to this PR

https://issues.redhat.com/browse/OCPBUGS-44882

• New Feature
• Enhancement
• Bug fix
• Tests
• Documentation
• CI/CD

What environments does this code impact?

• Automation (CI, tools, etc)
• Cloud
• Operator Managed Deployments
• None

How was this code tested?

• assisted-test-infra environment
• dev-scripts environment
• Reviewer's test appreciated
• Waiting for CI to do a full test run
• Manual (Elaborate on how it was tested)
• No tests needed

Tested creating a baremetal cluster and checking the generated installer configuration file.

Checklist

• Title and description added to both, commit and PR.
• Relevant issues have been associated (see CONTRIBUTING guide)
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

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]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhernand

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:

• OWNERS [jhernand]

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

[zaneb]

/lgtm

[zaneb]

/jira refresh
/cherry-pick release-4.18

[openshift-cherrypick-robot]

@zaneb: once the present PR merges, I will cherry-pick it on top of release-4.18 in a new PR and assign it to you.

In response to this:

/jira refresh
/cherry-pick release-4.18

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.

[openshift-ci-robot]

@zaneb: This pull request references Jira Issue OCPBUGS-44882, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.19.0) matches configured target version for branch (4.19.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira ([email protected]), skipping review request.

In response to this:

/jira refresh
/cherry-pick release-4.18

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.

[codecov]

Codecov Report

Attention: Patch coverage is 86.36364% with 3 lines in your changes missing coverage. Please review.

Project coverage is 68.80%. Comparing base (8dc67a1) to head (234490f).
Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/provider/external/base.go	0.00%	1 Missing ⚠️
internal/provider/nutanix/installConfig.go	0.00%	1 Missing ⚠️
internal/provider/vsphere/installConfig.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7028      +/-   ##
==========================================
+ Coverage   68.22%   68.80%   +0.58%     
==========================================
  Files         273      279       +6     
  Lines       39079    40187    +1108     
==========================================
+ Hits        26660    27652     +992     
- Misses      10007    10093      +86     
- Partials     2412     2442      +30     

Files with missing lines	Coverage Δ
internal/installcfg/builder/builder.go	79.59% <100.00%> (ø)
internal/provider/baremetal/installConfig.go	42.85% <100.00%> (ø)
internal/provider/registry/registry.go	63.51% <100.00%> (ø)
internal/provider/external/base.go	0.00% <0.00%> (ø)
internal/provider/nutanix/installConfig.go	0.00% <0.00%> (ø)
internal/provider/vsphere/installConfig.go	0.00% <0.00%> (ø)

... and 19 files with indirect coverage changes

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 8dc67a1 and 2 for PR HEAD a5ab953 in total

[jhernand]

/hold

I need to verify if we should take the NTP servers from the cluster or from the infrastructure environment.

[zaneb]

I need to verify if we should take the NTP servers from the cluster or from the infrastructure environment.

FWIW in ABI we pass them in the InfraEnv, so for ABI purposes that needs to make it all the way through to here. I'm not clear on whether the Cluster inherits the additionalNTPSources from its InfraEnv(s).

/lgtm cancel

[jhernand]

FWIW in ABI we pass them in the InfraEnv, so for ABI purposes that needs to make it all the way through to here. I'm not clear on whether the Cluster inherits the additionalNTPSources from its InfraEnv(s).

Yes, the new version of the patch uses the NTP sources for the cluster first, and if that is empty then it uses the ones from the infrastructure environments.

[zaneb]

/lgtm

[jhernand]

/retest

[gamli75]

@jhernand After it will be merged, please backport it to release-4.18 and release-ocm-2.12 branches

[jhernand]

/retest

[openshift-ci]

@jhernand: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/edge-e2e-oci-assisted	234490f	link	false	/test edge-e2e-oci-assisted
ci/prow/edge-e2e-nutanix-assisted-4-14	234490f	link	false	/test edge-e2e-nutanix-assisted-4-14
ci/prow/edge-e2e-nutanix-assisted	234490f	link	false	/test edge-e2e-nutanix-assisted
ci/prow/edge-e2e-oci-assisted-4-14	234490f	link	false	/test edge-e2e-oci-assisted-4-14

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.

[jhernand]

/retest

[gamli75]

@jhernand nutanix jobs are broken https://issues.redhat.com/browse/MGMT-19170
you can override it.
@adriengentil can you check the OCI tests? it looks broken as well

[jhernand]

Thanks @gamli75 . I don't have permisson to override jobs. Do you?

[jhernand]

/unhold

[openshift-ci-robot]

@jhernand: Jira Issue OCPBUGS-44882: All pull requests linked via external trackers have merged:

• openshift/assisted-service#7028

Jira Issue OCPBUGS-44882 has been moved to the MODIFIED state.

In response to this:

In OpenShift 4.18 a new additionalNTPServers field was added to the baremetal platform section of the install-config.yaml file . This needs to be populated with the NTP sources provided by the user in order to correctly set time before installation of new nodes using the baremetal provisioning flow.

List all the issues related to this PR

https://issues.redhat.com/browse/OCPBUGS-44882

• New Feature
• Enhancement
• Bug fix
• Tests
• Documentation
• CI/CD

What environments does this code impact?

• Automation (CI, tools, etc)
• Cloud
• Operator Managed Deployments
• None

How was this code tested?

• assisted-test-infra environment
• dev-scripts environment
• Reviewer's test appreciated
• Waiting for CI to do a full test run
• Manual (Elaborate on how it was tested)
• No tests needed

Tested creating a baremetal cluster and checking the generated installer configuration file.

Checklist

• Title and description added to both, commit and PR.
• Relevant issues have been associated (see CONTRIBUTING guide)
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

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-cherrypick-robot]

@zaneb: new pull request created: #7037

In response to this:

/jira refresh
/cherry-pick release-4.18

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.

[openshift-bot]

[ART PR BUILD NOTIFIER]

Distgit: ose-agent-installer-api-server
This PR has been included in build ose-agent-installer-api-server-container-v4.19.0-202411282238.p0.g160a229.assembly.stream.el9.
All builds following this will include this PR.

[jhernand]

/cherry-pick release-ocm-2.12

[openshift-cherrypick-robot]

@jhernand: new pull request created: #7038

In response to this:

/cherry-pick release-ocm-2.12

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.