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

The NTP_SERVER configuration generated from the minigraph doesn't meet the new schema requirements. #17906

Closed
oleksandrivantsiv opened this issue Jan 26, 2024 · 6 comments · Fixed by #18736 or #18981
Assignees
Labels
Triaged this issue has been triaged

Comments

@oleksandrivantsiv
Copy link
Collaborator

Description

The NTP_SERVER configuration generated from the minigraph doesn't meet the new schema requirements.

The issue is caused by the changes introduced in [NTP] Add NTP extended configuration PR. The PR updates the config DB schema and makes the NTP configuration generated from the minigraph incompatible.

Steps to reproduce the issue:

  1. Add NTP servers configuration to the minigraph file:
          <a:DeviceProperty>
            <a:Name>NtpResources</a:Name>
            <a:Reference i:nil="true"/>
            <a:Value>10.210.25.32;10.75.202.2</a:Value>
          </a:DeviceProperty>
  1. Run sonic-cfggen utility generate config DB configuration from the minigraph:
sonic-cfggen -m /etc/sonic/minigraph.xml  --print-data

Describe the results you received:

The utility generates the NTP_SERVER configuration in the following format:

    "NTP_SERVER": {
        "10.210.25.32": {},
        "10.75.202.2": {}
    },

As a result, the NTP feature ignores this configuration, and the time synchronization doesn't work:

# show ntp
MGMT_VRF_CONFIG is not present.
unsynchronised
   polling server every 1 s

Describe the results you expected:

The NTP feature implementation expects to have an extended configuration in the format like this:

    "NTP_SERVER": {
        "internal_ntp_server1": {
            "association_type": "server",
            "iburst": "on",
            "admin_state": "enabled",
            "version": 3,
            "resolve_as": "10.210.25.32"
       },
        "internal_ntp_server2": {
            "association_type": "server",
            "iburst": "on",
            "admin_state": "enabled",
            "version": 3,
            "resolve_as": "10.75.202.2"
       },
    },

The YANG model for the configuration is defined here sonic-ntp.yang

Output of show version:

# show version

SONiC Software Version: SONiC.master.560-c274be2e5_Internal
SONiC OS Version: 12
Distribution: Debian 12.4
Kernel: 6.1.0-11-2-amd64
Build commit: c274be2e5
Build date: Mon Jan 22 21:57:27 UTC 2024
Built by: sw-r2d2-bot@r-build-sonic-ci02-244

Platform: x86_64-mlnx_msn4700-r0
HwSKU: Mellanox-SN4700-O28
ASIC: mellanox
ASIC Count: 1
Serial Number: MT2247XZ0588
Model Number: MSN4700-WS2F
Hardware Revision: A1
Uptime: 02:17:59 up  1:25,  1 user,  load average: 0.41, 0.32, 0.35
Date: Fri 26 Jan 2024 02:17:59

Output of show techsupport:

(paste your output here or download and attach the file here )

Additional information you deem important (e.g. issue happens only occasionally):

@gechiang
Copy link
Collaborator

@fastiuk , please help fix this as the PR you raised seems to be the root cause of this issue.

@gechiang gechiang added the Triaged this issue has been triaged label Feb 14, 2024
@fastiuk
Copy link
Contributor

fastiuk commented Mar 6, 2024

@fastiuk , please help fix this as the PR you raised seems to be the root cause of this issue.

@gechiang do you know what is the due date to fix this issue?

@gechiang
Copy link
Collaborator

@fastiuk , I would say as soon as possible as this is a regression and impacting the community.

@saiarcot895
Copy link
Contributor

This issue is present on both master and 202311 branch.

@fastiuk
Copy link
Contributor

fastiuk commented Apr 1, 2024

Due date is Apr 12

@fastiuk
Copy link
Contributor

fastiuk commented Apr 8, 2024

WIP on that issue. Any way to move it to the "In progress" state, or there is no such thing on GitHub?

qiluo-msft pushed a commit that referenced this issue May 15, 2024
fixes #17906

#### Why I did it
To fix NTP config generation from the minigraph and save backward compatability

#### How I did it
Align `ntp.conf.j2` template to generate config out of empty `NTP_SERVER` DB configuration

#### How to verify it
Out of that NTP_SERVER configuration:
```json
{
  "10.210.25.32": {},
  "10.75.202.2": {}
}
```

The next config in `ntp.conf` file should be produced:
```
server 10.210.25.32
restrict 10.210.25.32 kod limited nomodify notrap noquery nopeer

server 10.75.202.2
restrict 10.75.202.2 kod limited nomodify notrap noquery nopeer
```
fastiuk added a commit to fastiuk/sonic-buildimage that referenced this issue May 16, 2024
fixes sonic-net#17906

#### Why I did it
To fix NTP config generation from the minigraph and save backward compatability

#### How I did it
Align `ntp.conf.j2` template to generate config out of empty `NTP_SERVER` DB configuration

#### How to verify it
Out of that NTP_SERVER configuration:
```json
{
  "10.210.25.32": {},
  "10.75.202.2": {}
}
```

The next config in `ntp.conf` file should be produced:
```
server 10.210.25.32
restrict 10.210.25.32 kod limited nomodify notrap noquery nopeer

server 10.75.202.2
restrict 10.75.202.2 kod limited nomodify notrap noquery nopeer
```
fastiuk added a commit to fastiuk/sonic-buildimage that referenced this issue May 16, 2024
fixes sonic-net#17906

To fix NTP config generation from the minigraph and save backward compatability

Align `ntp.conf.j2` template to generate config out of empty `NTP_SERVER` DB configuration

Out of that NTP_SERVER configuration:
```json
{
  "10.210.25.32": {},
  "10.75.202.2": {}
}
```

The next config in `ntp.conf` file should be produced:
```
server 10.210.25.32
restrict 10.210.25.32 kod limited nomodify notrap noquery nopeer

server 10.75.202.2
restrict 10.75.202.2 kod limited nomodify notrap noquery nopeer
```

Signed-off-by: Yevhen Fastiuk <[email protected]>
yxieca pushed a commit that referenced this issue May 28, 2024
fixes #17906

To fix NTP config generation from the minigraph and save backward compatability

Align `ntp.conf.j2` template to generate config out of empty `NTP_SERVER` DB configuration

Out of that NTP_SERVER configuration:
```json
{
  "10.210.25.32": {},
  "10.75.202.2": {}
}
```

The next config in `ntp.conf` file should be produced:
```
server 10.210.25.32
restrict 10.210.25.32 kod limited nomodify notrap noquery nopeer

server 10.75.202.2
restrict 10.75.202.2 kod limited nomodify notrap noquery nopeer
```

Signed-off-by: Yevhen Fastiuk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Triaged this issue has been triaged
Projects
None yet
4 participants