Revert "[sonic-package-manager] support sonic-cli-gen and packages wi… #1972

prgeor · 2021-12-16T07:19:57Z

This reverts commit f5e5a56.

Getting following build failure while updating sonic-utilities submodule PR9467

2021-12-13T11:10:48.6811881Z "gbsyncd": {
2021-12-13T11:10:48.6812291Z         "repository": "docker-gbsyncd-vs",
2021-12-13T11:10:48.6812715Z         "description": "SONiC gbsyncd package",
2021-12-13T11:10:48.6813182Z         "default-reference": "1.0.0",
2021-12-13T11:10:48.6813725Z         "installed-version": "1.0.0",
2021-12-13T11:10:48.6814236Z         "built-in": true,
2021-12-13T11:10:48.6814867Z         "installed": true
2021-12-13T11:10:48.6815196Z     }
2021-12-13T11:10:48.6815357Z 
2021-12-13T11:10:48.6815555Z }
2021-12-13T11:10:48.6815934Z + '[' 0 '!=' 0 ']'
2021-12-13T11:10:48.6816655Z + sudo cp files/build_templates/docker_image_ctl.j2 ./fsroot-vs/usr/share/sonic/templates/docker_image_ctl.j2
2021-12-13T11:10:48.6817592Z + sudo LANG=C DOCKER_HOST= chroot ./fsroot-vs /usr/local/bin/generate_shutdown_order.py
2021-12-13T11:10:48.6819243Z libyang[0]: Value "{% if not (DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined and DEVICE_METADATA['localhost']['type'] != 'ToRRouter') %}enabled{% else %}disabled{% endif %}" does not satisfy the constraint "enabled|disabled|always_enabled|always_disabled" (range, length, or pattern). (path: /sonic-feature:sonic-feature/FEATURE/FEATURE_LIST[name='dhcp_relay']/state)
2021-12-13T11:10:48.6821253Z sonic_yang(3):Data Loading Failed:Value "{% if not (DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined and DEVICE_METADATA['localhost']['type'] != 'ToRRouter') %}enabled{% else %}disabled{% endif %}" does not satisfy the constraint "enabled|disabled|always_enabled|always_disabled" (range, length, or pattern).
2021-12-13T11:10:48.6822091Z Data Loading Failed
2021-12-13T11:10:48.6823172Z Value "{% if not (DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined and DEVICE_METADATA['localhost']['type'] != 'ToRRouter') %}enabled{% else %}disabled{% endif %}" does not satisfy the constraint "enabled|disabled|always_enabled|always_disabled" (range, length, or pattern).
2021-12-13T11:10:48.6823980Z Traceback (most recent call last):
2021-12-13T11:10:48.6824541Z   File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 1065, in loadData
2021-12-13T11:10:48.6825037Z     self.root = self.ctx.parse_data_mem(dumps(self.xlateJson), \
2021-12-13T11:10:48.6825605Z   File "/usr/lib/python3/dist-packages/yang.py", line 2604, in parse_data_mem
2021-12-13T11:10:48.6826068Z     return _yang.Context_parse_data_mem(self, data, format, options)
2021-12-13T11:10:48.6827257Z RuntimeError: Value "{% if not (DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined and DEVICE_METADATA['localhost']['type'] != 'ToRRouter') %}enabled{% else %}disabled{% endif %}" does not satisfy the constraint "enabled|disabled|always_enabled|always_disabled" (range, length, or pattern).
2021-12-13T11:10:48.6827923Z 
2021-12-13T11:10:48.6831352Z During handling of the above exception, another exception occurred:
2021-12-13T11:10:48.6831689Z 
2021-12-13T11:10:48.6831968Z Traceback (most recent call last):
2021-12-13T11:10:48.6832674Z   File "/usr/local/lib/python3.9/dist-packages/config/config_mgmt.py", line 61, in __init__
2021-12-13T11:10:48.6833344Z     self.__init_sonic_yang()
2021-12-13T11:10:48.6833926Z   File "/usr/local/lib/python3.9/dist-packages/config/config_mgmt.py", line 80, in __init_sonic_yang
2021-12-13T11:10:48.6834407Z     self.sy.loadData(self.configdbJsonIn)
2021-12-13T11:10:48.6834958Z   File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 1072, in loadData
2021-12-13T11:10:48.6835465Z     raise SonicYangException("Data Loading Failed\n{}".format(str(e)))
2021-12-13T11:10:48.6835827Z sonic_yang_ext.SonicYangException: Data Loading Failed
2021-12-13T11:10:48.6836962Z Value "{% if not (DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined and DEVICE_METADATA['localhost']['type'] != 'ToRRouter') %}enabled{% else %}disabled{% endif %}" does not satisfy the constraint "enabled|disabled|always_enabled|always_disabled" (range, length, or pattern).
2021-12-13T11:10:48.6837604Z 
2021-12-13T11:10:48.6837951Z During handling of the above exception, another exception occurred:
2021-12-13T11:10:48.6838154Z 
2021-12-13T11:10:48.6838377Z Traceback (most recent call last):
2021-12-13T11:10:48.6838835Z   File "/usr/local/bin/generate_shutdown_order.py", line 15, in <module>
2021-12-13T11:10:48.6839142Z     main()
2021-12-13T11:10:48.6839425Z   File "/usr/local/bin/generate_shutdown_order.py", line 8, in main
2021-12-13T11:10:48.6839959Z     manager = PackageManager.get_manager()
2021-12-13T11:10:48.6840567Z   File "/usr/local/lib/python3.9/dist-packages/sonic_package_manager/manager.py", line 1012, in get_manager
2021-12-13T11:10:48.6841085Z     cfg_mgmt = config_mgmt.ConfigMgmt(source=INIT_CFG_JSON)
2021-12-13T11:10:48.6841660Z   File "/usr/local/lib/python3.9/dist-packages/config/config_mgmt.py", line 65, in __init__
2021-12-13T11:10:48.6842296Z     raise Exception('ConfigMgmt Class creation failed')
2021-12-13T11:10:48.6842704Z Exception: ConfigMgmt Class creation failed
2021-12-13T11:10:48.6842987Z + clean_proc
2021-12-13T11:10:48.6843210Z + sudo umount /proc
2021-12-13T11:10:48.6843443Z + clean_sys
2021-12-13T11:10:48.6844659Z + sudo chroot ./fsroot-vs umount /sys/fs/cgroup/blkio /sys/fs/cgroup/cpu /sys/fs/cgroup/cpu,cpuacct /sys/fs/cgroup/cpuacct /sys/fs/cgroup/cpuset /sys/fs/cgroup/devices /sys/fs/cgroup/freezer /sys/fs/cgroup/hugetlb /sys/fs/cgroup/memory /sys/fs/cgroup/net_cls /sys/fs/cgroup/net_cls,net_prio /sys/fs/cgroup/net_prio /sys/fs/cgroup/perf_event /sys/fs/cgroup/pids /sys/fs/cgroup/rdma /sys/fs/cgroup/systemd /sys/fs/cgroup /sys
2021-12-13T11:10:48.6845590Z umount: /sys/fs/cgroup/cpu: no mount point specified.
2021-12-13T11:10:48.6845957Z umount: /sys/fs/cgroup/cpu,cpuacct: no mount point specified.
2021-12-13T11:10:48.6846371Z umount: /sys/fs/cgroup/cpuacct: no mount point specified.
2021-12-13T11:10:48.6846735Z umount: /sys/fs/cgroup/net_cls: no mount point specified.
2021-12-13T11:10:48.6847096Z umount: /sys/fs/cgroup/net_cls,net_prio: no mount point specified.
2021-12-13T11:10:48.6847557Z umount: /sys/fs/cgroup/net_prio: no mount point specified.
2021-12-13T11:10:48.6847907Z umount: /sys/fs/cgroup/systemd: no mount point specified.
2021-12-13T11:10:48.6848187Z + true
2021-12-13T11:10:48.6848380Z + true
2021-12-13T11:10:48.6848793Z + sudo LANG=C chroot ./fsroot-vs umount /proc
2021-12-13T11:10:48.6849132Z + true
2021-12-13T11:10:48.6849547Z [  FAIL LOG END  ] [ target/sonic-vs.img.gz ]
2021-12-13T11:10:48.6850110Z make: *** [slave.mk:990: target/sonic-vs.img.gz] Error 1
2021-12-13T11:10:50.3928072Z make[1]: *** [Makefile.work:309: target/sonic-vs.img.gz] Error 2
2021-12-13T11:10:50.3928876Z make[1]: Leaving directory '/agent/_work/1/s'
2021-12-13T11:10:50.3929425Z make: *** [Makefile:33: target/sonic-vs.img.gz] Error 2
2021-12-13T11:10:50.4767031Z ##[error]Bash exited with code '2'.
2021-12-13T11:10:50.5440899Z ##[section]Finishing: Build sonic image

…th YANG model (sonic-net#1650)" This reverts commit f5e5a56.

prgeor · 2021-12-16T07:23:25Z

@stepanblyschak i had to revert your PR1650 to fix this build error https://dev.azure.com/mssonic/be1b070f-be15-4154-aade-b1d3bfb17054/_apis/build/builds/57684/logs/141. Please check your changes against latest master build.

stepanblyschak · 2021-12-16T09:34:57Z

@prgeor The failure indicates that there is an issue with init_cfg.json, the problem is that init_cfg.json is not aligned to YANG. The failure is not related to an issue in the code change in 1650,

2021-12-13T11:10:48.6823172Z Value "{% if not (DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined and DEVICE_METADATA['localhost']['type'] != 'ToRRouter') %}enabled{% else %}disabled{% endif %}" does not satisfy the constraint "enabled|disabled|always_enabled|always_disabled" (range, length, or pattern).

This means that (https://github.com/Azure/sonic-buildimage/blob/master/files/build_templates/init_cfg.json.j2#L39):

"state": "{% if not (DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined and DEVICE_METADATA['localhost']['type'] != 'ToRRouter') %}enabled{% else %}disabled{% endif %}"

is not accepted by YANG validation, which makes sense because according to YANG model for FEATURE table (https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-feature.yang#L17) the "state" can only be "enabled", "disabled", "always_enabled", "always_disabled".

I am not familiar why the change that made "state" value a jinja template, but if that the new schema for it then we need to fix YANG model for FEATURE table and make the "state" field of type "string".

@lguohan @praveen-li @arlakshm @shlomibitton @tahmed-dev

praveen-li · 2021-12-16T17:48:57Z

Config is wrong, we will not be able to fix it using yang. Jinja must be compiled properly. Config in log: ``` 2021-12-13T10:25:11.7331858Z "teamd": { 2021-12-13T10:25:11.7332256Z "state": "enabled", 2021-12-13T10:25:11.7333722Z "has_timer" : false, 2021-12-13T10:25:11.8013978Z "has_global_scope": false, 2021-12-13T10:25:11.8014491Z "has_per_asic_scope": true, 2021-12-13T10:25:11.8015006Z "auto_restart": "enabled", 2021-12-13T10:25:11.8015724Z "high_mem_alert": "disabled" 2021-12-13T10:25:11.8016059Z }, 2021-12-13T10:25:11.8016355Z "dhcp_relay": { 2021-12-13T10:25:11.8018638Z "state": "{% if not (DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined and DEVICE_METADATA['localhost']['type'] != 'ToRRouter') %}enabled{% else %}disabled{% endif %}",<<<< 2021-12-13T10:25:11.8020301Z "has_timer" : false, 2021-12-13T10:25:11.8020671Z "has_global_scope": false, 2021-12-13T10:25:11.8021013Z "has_per_asic_scope": false, 2021-12-13T10:25:11.8021369Z "auto_restart": "enabled", 2021-12-13T10:25:11.8021707Z "high_mem_alert": "disabled" 2021-12-13T10:25:11.8022006Z }, Thanks, Regards Praveen

…

qiluo-msft · 2021-12-16T22:14:26Z

@shlomibitton @stepanblyschak Could you discuss and identify which component need fix?

stepanblyschak · 2021-12-17T14:27:13Z

@qiluo-msft I created this PR sonic-net/sonic-buildimage#9587 that I propose as a fix.
Unfortunatelly, I am not sure what should be the right fix.
The reason is that we have added an ability to dynamically render a part of configuration from the feature "state" field - sonic-net/sonic-buildimage#6700.
However, we do not have this scenario accounted in the YANG model - https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-feature.yang#L17.

To fix this, my PR relaxes the constraint on the "state" field, so it can be any string, as it might be difficult to validate that the "state" field is at least syntactically valid jinja template in the YANG model.

On the other hand, the YANG validation will not raise any errors on really incorrect "state" field values - so that's my concern.

I am wondering why sonic-net/sonic-buildimage#6700 was the proposed solution and what is the use case? It looks like it complicates the schema a lot and it is kind of introducing a new concept of dynamic configuration in sonic, it also compliates the hostcfgd. Couldn't it be solved on the configuration generation tools (minigraph, sonic-cfggen). It is much simpler and makes more sense - if the config we are generating is for the DualTor then enable mux service if not make it always disabled. Is there an HLD for this feature? Maybe we can discuss about a different solution instead of relaxing YANG validation?

Unfortunatelly, the PR that is beeing reverted by this PR was developed half a year ago when there was no YANG/config DB issues but it got merged recently and exposed all those issues introduced since then.

prgeor · 2021-12-17T14:40:30Z

@stepanblyschak for now i need to roll back your pr since more than a dozen PRs are awaiting to be available in master for more than a month now - sonic-net/sonic-buildimage#9467

shlomibitton · 2021-12-19T08:44:39Z

@qiluo-msft @stepanblyschak
The change for the dhcp_relay feature state introduced on this PR: sonic-net/sonic-buildimage#7789
The motivation for this change is to disable dhcp_relay by default for non ToRRouter switches from init_cfg.json.
With this approach, if the user want to enable the feature for non ToRRouter switches, manual enablement is required by the 'feature' configuration and then the COPP manager will hook dhcp packets to the CPU.
This is to keep the current approach for MSFT production issue with dhcp relay for non ToRRouter switched and allow the user to decide if to use it or not.
Before this change, it was impossible to enable the dhcp_relay feature, even when the feature was running the Copp manager did not register this trap group.

…kages with YANG model (sonic-net#1650)" (sonic-net#1972)" This reverts commit fe00bbf.

…th YANG model (#1650)" (#1972) This reverts commit f5e5a56.

…kages with YANG model (sonic-net#1650)" (sonic-net#1972)" This reverts commit fe00bbf.

…kages with YANG model (#1650)" (#1972)" (#1994) This reverts commit fe00bbf. - What I did Revert previous revert, since the proposed fix has been merged - sonic-net/sonic-buildimage#9587 - How I did it Revert the revert. - How to verify it Run build an on the switch.

…kages with YANG model (sonic-net#1650)" (sonic-net#1972)" (sonic-net#1994) This reverts commit fe00bbf. - What I did Revert previous revert, since the proposed fix has been merged - sonic-net/sonic-buildimage#9587 - How I did it Revert the revert. - How to verify it Run build an on the switch.

Revert "[sonic-package-manager] support sonic-cli-gen and packages wi…

747ef83

…th YANG model (sonic-net#1650)" This reverts commit f5e5a56.

prgeor requested a review from qiluo-msft December 16, 2021 07:20

qiluo-msft approved these changes Dec 17, 2021

View reviewed changes

prgeor merged commit fe00bbf into sonic-net:master Dec 17, 2021

aravindmani-1 mentioned this pull request Dec 18, 2021

[submodule] Update sonic-utilities submodule sonic-net/sonic-buildimage#9467

Closed

5 tasks

stepanblyschak mentioned this pull request Dec 20, 2021

[init_cfg.json][YANG] The initial configuration /etc/sonic/init_cfg.json does not pass YANG validation sonic-net/sonic-buildimage#9598

Open

stepanblyschak added a commit to stepanblyschak/sonic-utilities that referenced this pull request Dec 30, 2021

Revert "Revert "[sonic-package-manager] support sonic-cli-gen and pac…

ffdf73a

…kages with YANG model (sonic-net#1650)" (sonic-net#1972)" This reverts commit fe00bbf.

stepanblyschak mentioned this pull request Dec 30, 2021

Revert "Revert "[sonic-package-manager] support sonic-cli-gen and pac… #1994

Merged

judyjoseph pushed a commit that referenced this pull request Jan 5, 2022

Revert "[sonic-package-manager] support sonic-cli-gen and packages wi…

c48c363

…th YANG model (#1650)" (#1972) This reverts commit f5e5a56.

DavidZagury added a commit to DavidZagury/sonic-utilities that referenced this pull request Jan 12, 2022

Revert "Revert "[sonic-package-manager] support sonic-cli-gen and pac…

d0ebe63

…kages with YANG model (sonic-net#1650)" (sonic-net#1972)" This reverts commit fe00bbf.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert "[sonic-package-manager] support sonic-cli-gen and packages wi… #1972

Revert "[sonic-package-manager] support sonic-cli-gen and packages wi… #1972

prgeor commented Dec 16, 2021 •

edited by qiluo-msft

Loading

prgeor commented Dec 16, 2021

stepanblyschak commented Dec 16, 2021

praveen-li commented Dec 16, 2021 via email

qiluo-msft commented Dec 16, 2021

stepanblyschak commented Dec 17, 2021

prgeor commented Dec 17, 2021

shlomibitton commented Dec 19, 2021 •

edited

Loading

Revert "[sonic-package-manager] support sonic-cli-gen and packages wi… #1972

Revert "[sonic-package-manager] support sonic-cli-gen and packages wi… #1972

Conversation

prgeor commented Dec 16, 2021 • edited by qiluo-msft Loading

prgeor commented Dec 16, 2021

stepanblyschak commented Dec 16, 2021

praveen-li commented Dec 16, 2021 via email

qiluo-msft commented Dec 16, 2021

stepanblyschak commented Dec 17, 2021

prgeor commented Dec 17, 2021

shlomibitton commented Dec 19, 2021 • edited Loading

prgeor commented Dec 16, 2021 •

edited by qiluo-msft

Loading

shlomibitton commented Dec 19, 2021 •

edited

Loading