openconfig / featureprofiles

Feature Profiles are groups of OpenConfig paths and tests which verify their behavior
Apache License 2.0
48 stars 138 forks source link

gNMI-1.15: gnmi.Replace is sending state in the config value #2465

Closed trathod1 closed 9 months ago

trathod1 commented 9 months ago

Describe the bug After commit of https://github.com/openconfig/featureprofiles/pull/2366, it is observed that gnmi.Replace at the container level is sending state information also in config.

@greg-dennis Do we need to modify featureprofiles script to accomodate PR-2366 changes ?

To eloborate, here is the output of following 2 modified lines from the script to debug -

       fptest.LogQuery(t, "NetworkInstance", networkInstancesQuery, &NetworkInstances{NetworkInstance: config.NetworkInstance})
       gnmi.Replace(t, dev, networkInstancesQuery, &NetworkInstances{NetworkInstance: config.NetworkInstance})

Output -

    gnmi_set_test.go:253: Config for NetworkInstance at /network-instances:
        {
          "openconfig-network-instance:network-instance": [
            {
              "config": {
                "name": "DEFAULT",
                "type": "openconfig-network-instance-types:DEFAULT_INSTANCE"
              },
              "name": "DEFAULT"
            },
            {
              "config": {
                "name": "inband-mgmt",
                "type": "openconfig-network-instance-types:L3VRF"
              },
              "interfaces": {
                "interface": [
                  {
                    "config": {
                      "id": "lag1.1"
                    },
                    "id": "lag1.1"
                  },
                  {
                    "config": {
                      "id": "lag2.1"
                    },
                    "id": "lag2.1"
                  },
                  {
                    "config": {
                      "id": "lo0.2"
                    },
                    "id": "lo0.2"
                  }
                ]
              },
              "name": "inband-mgmt",
              "protocols": {
                "protocol": [
                  {
                    "config": {
                      "identifier": "openconfig-policy-types:STATIC",
                      "name": "static"
                    },
                    "identifier": "openconfig-policy-types:STATIC",
                    "name": "static",
                    "static-routes": {
                      "static": [
                        {
                          "config": {
                            "prefix": "0.0.0.0/0"
                          },
                          "next-hops": {
                            "next-hop": [
                              {
                                "config": {
                                  "index": "1",
                                  "next-hop": "10.61.62.168"
                                },
                                "index": "1"
                              }
                            ]
                          },
                          "prefix": "0.0.0.0/0"
                        },
                        {
                          "config": {
                            "prefix": "::/0"
                          },
                          "next-hops": {
                            "next-hop": [
                              {
                                "config": {
                                  "index": "1",
                                  "next-hop": "2607:f8b0:8007:51c0::b4"
                                },
                                "index": "1"
                              }
                            ]
                          },
                          "prefix": "::/0"
                        }
                      ]
                    }
                  }
                ]
              }
            }
          ]
        }
2023/12/08 10:44:58 Test output written: /var/www/html/featureprofiles/logs/manual_run/08-12-2023/TestSwapIPs_ContainerOp_Config_for_NetworkInstance_at__network-instances.10:44:58.2033055919.json
*** PROPERTY: test_output0 -> TestSwapIPs_ContainerOp_Config_for_NetworkInstance_at__network-instances.10:44:58.2033055919.json
    gnmi_set_test.go:253: Replace(t) on dut(MVSRLHW20-DUT20) at /network-instances: Replace(t) at path origin:"openconfig"  elem:{name:"network-instances"}: rpc error: code = NotFound desc = error on request {
        prefix:  {
          target:  "MVSRLHW20-DUT20"
        }
        replace:  {
          path:  {
            origin:  "openconfig"
            elem:  {
              name:  "network-instances"
            }
          }
          val:  {
            json_ietf_val:  "{\n  \"openconfig-network-instance:network-instance\": [\n    {\n      \"name\": \"DEFAULT\",\n      \"state\": {\n        \"name\": \"DEFAULT\",\n        \"type\": \"openconfig-network-instance-types:DEFAULT_INSTANCE\"\n      }\n    },\n    {\n      \"interfaces\": {\n        \"interface\": [\n          {\n            \"id\": \"lag1.1\",\n            \"state\": {\n              \"id\": \"lag1.1\"\n            }\n          },\n          {\n            \"id\": \"lag2.1\",\n            \"state\": {\n              \"id\": \"lag2.1\"\n            }\n          },\n          {\n            \"id\": \"lo0.2\",\n            \"state\": {\n              \"id\": \"lo0.2\"\n            }\n          }\n        ]\n      },\n      \"name\": \"inband-mgmt\",\n      \"protocols\": {\n        \"protocol\": [\n          {\n            \"identifier\": \"openconfig-policy-types:STATIC\",\n            \"name\": \"static\",\n            \"state\": {\n              \"identifier\": \"openconfig-policy-types:STATIC\",\n              \"name\": \"static\"\n            },\n            \"static-routes\": {\n              \"static\": [\n                {\n                  \"next-hops\": {\n                    \"next-hop\": [\n                      {\n                        \"index\": \"1\",\n                        \"state\": {\n                          \"index\": \"1\",\n                          \"next-hop\": \"10.61.62.168\"\n                        }\n                      }\n                    ]\n                  },\n                  \"prefix\": \"0.0.0.0/0\",\n                  \"state\": {\n                    \"prefix\": \"0.0.0.0/0\"\n                  }\n                },\n                {\n                  \"next-hops\": {\n                    \"next-hop\": [\n                      {\n                        \"index\": \"1\",\n                        \"state\": {\n                          \"index\": \"1\",\n                          \"next-hop\": \"2607:f8b0:8007:51c0::b4\"\n                        }\n                      }\n                    ]\n                  },\n                  \"prefix\": \"::/0\",\n                  \"state\": {\n                    \"prefix\": \"::/0\"\n                  }\n                }\n              ]\n            }\n          }\n        ]\n      },\n      \"state\": {\n        \"name\": \"inband-mgmt\",\n        \"type\": \"openconfig-network-instance-types:L3VRF\"\n      }\n    }\n  ]\n}"
          }
        }
        }: Schema '/network-instances/network-instance' has no child with name 'state'. Options are [config, interfaces, policy-forwarding, protocols]: Parse error on line 83:       },
        Input:
        '{
          "openconfig-network-instance:network-instance": [
            {
              "name": "DEFAULT",
              "state": {
                "name": "DEFAULT",
                "type": "openconfig-network-instance-types:DEFAULT_INSTANCE"
              }
            },
            {
              "interfaces": {
                "interface": [
                  {
                    "id": "lag1.1",
                    "state": {
                      "id": "lag1.1"
                    }
                  },
                  {
                    "id": "lag2.1",
                    "state": {
                      "id": "lag2.1"
                    }
                  },
               ...'

To Reproduce Executing script feature/system/gnmi/set/tests/gnmi_set_test/gnmi_set_test.go shows the failure.

Expected behavior Expecting ygnmi not to alter the config during replace like it was working before https://github.com/openconfig/featureprofiles/pull/2366.

Logs https://gist.github.com/trathod1/3a9b9b0dd20f39f870535dc9a2e6dbad

sachendras commented 9 months ago

Assigned to @wenovus since the issue manifested after https://github.com/openconfig/ygnmi/commit/c32508090ccc133d87048801a4a320427f2e430d

wenovus commented 9 months ago

Sorry was away, taking a look now.

wenovus commented 9 months ago

Thanks @sachendras for pinpointing the culprit PR.

gnmi_set_test was depending on a very particular and non-intuitive marshal behaviour for schemaless queries that was changed in the most recent ygnmi release.

https://github.com/openconfig/featureprofiles/pull/2475 should fix the issue.

What's the best way to test these changes and unblock progress?