Skip to content

feat: Implement API to set NTP servers#87

Open
hanyux-nv wants to merge 2 commits into
NVIDIA:mainfrom
hanyux-nv:ntp_server_config
Open

feat: Implement API to set NTP servers#87
hanyux-nv wants to merge 2 commits into
NVIDIA:mainfrom
hanyux-nv:ntp_server_config

Conversation

@hanyux-nv

Copy link
Copy Markdown

Added a new Redfish API method set_ntp_servers that configures a list of NTP servers of the BMC.

See issue #548.

Signed-off-by: Felicity Xu <hanyux@nvidia.com>
spydaNVIDIA
spydaNVIDIA previously approved these changes Jun 9, 2026

@spydaNVIDIA spydaNVIDIA left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

lgtm!

@krish-nvidia

Copy link
Copy Markdown

For Dells, you have to issue the following redfish call. Out of experience, setting them in NetworkProtocols doesn't work 😅

curl -sku "username:password" \
  -X PATCH \
  -H "Content-Type: application/json" \
  -d '{
    "Attributes": {
      "NTPConfigGroup.1.NTPEnable": "Enabled",
      "NTPConfigGroup.1.NTP1": "pool.ntp.org",
      "NTPConfigGroup.1.NTP2": "time.google.com"
    }
  }' \
  "https://<iDRAC_IP>/redfish/v1/Managers/iDRAC.Embedded.1/Attributes"

@krish-nvidia krish-nvidia left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See comment above ^

@krish-nvidia

Copy link
Copy Markdown

For HPEs, you have to use /redfish/v1/Managers/1/DateTime to set the NTP servers. Other vendors might require other paths as well so I'd try them out on each machine to make sure it works.

Signed-off-by: Felicity Xu <hanyux@nvidia.com>
Comment thread src/ami.rs
.client
.patch_with_if_match("UpdateService", body)
.await
fn set_ntp_servers<'a>(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

AMI servers require PATCH's to be sent with an If-Match header. Instead of delegating it to the standard implementation, do the following:

        if servers.is_empty() {
            return Ok(());
        }
        let url = format!("Managers/{}/NetworkProtocol", self.s.manager_id());
        let body = HashMap::from([(
            "NTP",
            serde_json::json!({
                "NTPServers": servers,
                "ProtocolEnabled": true,
            }),
        )]);
        self.s.client.patch_with_if_match(&url, body).await

Comment thread src/dell.rs
}

let mut attrs = HashMap::new();
attrs.insert("NTPConfigGroup.1.NTPEnable", "Enabled");

@krish-nvidia krish-nvidia Jun 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unused slots keep stale values when fewer than 3 servers are passed into set_ntp_servers. We should explicitly blank them out so the set is authoritative. Something like:

let mut attrs = HashMap::from([("NTPConfigGroup.1.NTPEnable", "Enabled")]);
const NTP_KEYS: [&str; 3] = [
    "NTPConfigGroup.1.NTP1",
    "NTPConfigGroup.1.NTP2",
    "NTPConfigGroup.1.NTP3",
];
for (i, key) in NTP_KEYS.into_iter().enumerate() {
    // blank unused slots so the set is authoritative
    attrs.insert(key, servers.get(i).map_or("", String::as_str));
}

Comment thread src/hpe.rs
return Ok(());
}

// iLO supports at most 2 static NTP servers; extra entries are ignored.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can do something like this instead:
let static_ntp_servers: Vec<String> = servers.iter().take(2).cloned().collect();

Also we should log when a user passes in more than 2 servers:

if servers.len() > 2 {
    tracing::warn!("iLO supports at most 2 static NTP servers; ignoring {} extra", servers.len() - 2);
}

Comment thread src/standard.rs
return Ok(());
}

let url = format!("Managers/{}/NetworkProtocol", self.manager_id(),);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: extra comma next to self.manager_id()

@krish-nvidia

Copy link
Copy Markdown

Also, delta_powershelf.rs is missing set_ntp_servers(), the library won't compile otherwise.

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.

3 participants