Skip to content

[sonic-package-manager] Fix YANG validation failure on upgrade when feature has constraints in YANG model on FEATURE table#2933

Merged
bingwang-ms merged 2 commits intosonic-net:masterfrom
stepanblyschak:spm-upgrade-feautre-yang
Nov 20, 2023
Merged

[sonic-package-manager] Fix YANG validation failure on upgrade when feature has constraints in YANG model on FEATURE table#2933
bingwang-ms merged 2 commits intosonic-net:masterfrom
stepanblyschak:spm-upgrade-feautre-yang

Conversation

@stepanblyschak
Copy link
Copy Markdown
Contributor

@stepanblyschak stepanblyschak commented Jul 31, 2023

In case feature's YANG model has a constraint on FEATURE table:

must "current() = \"disabled\" or /feature:sonic-feature/feature:FEATURE/feature:FEATURE_LIST[feature:name=\"Y\"]/feature:state = \"enabled\"" {
    error-message "X can only be set when is Y enabled";
}

it will fail to upgrade due to transient disablement of the feature by the upgrade process.

The upgrade process looks like this:

  1. Disable feature via FEATURE table and wait for container to go down.
  2. Remove all configuration files generated for this feature.
  3. Create new configuration files for new docker image.
  4. Merge new init_cfg from the new docker image and perform YANG validation.
  5. Enable feature via FEATURE table.
  6. Remove old docker image.

Since there is a transient enable/disable of the feature and YANG validation in the middle having the above constrains in YANG model would lead to an upgrade failure.

What I did

I fixed an issue that leads to upgrade failure for some extensions.

How I did it

Replaces setting FEATURE state with systemctl commands.

How to verify it

Given the above example, verify that the upgrade process passes successfully.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

…eature has constraints in YANG model on FEATURE table

In case feature's YANG model has a constraint on FEATURE table:

```
must "current() = \"disabled\" or /feature:sonic-feature/feature:FEATURE/feature:FEATURE_LIST[feature:name=\"Y\"]/feature:state = \"enabled\"" {
    error-message "X can only be set when is Y enabled";
}
```

it will fail to upgrade due to transient disablement of the feature by
the upgrade process.

The upgrade process looks like this:
1. Disable feature via FEATURE table and wait for container to go down.
2. Remove all configuration files generated for this feature.
3. Create new configuration files for new docker image.
4. Merge new init_cfg from the new docker image and perform YANG
   validation.
5. Enable feature via FEATURE table.
6. Remove old docker image.

Since there is a transient enable/disable of the feature and YANG
validation in the middle having the above constrains in YANG model would
lead to an upgrade failure.

I attempt to fix this by executing systemctl commands directly without
the modification of the FEATURE table.

Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
@liat-grozovik
Copy link
Copy Markdown
Collaborator

@qiluo-msft could you help to review or assign someone?

@liat-grozovik
Copy link
Copy Markdown
Collaborator

@stepanblyschak please handle the easyCLA

@stepanblyschak
Copy link
Copy Markdown
Contributor Author

@liat-grozovik I don't know what is needed from my side to handle EasyCLA. It looks like it hanged. @liat-grozovik @volodymyrsamotiy @qiluo-msft Could you please help?

@liat-grozovik
Copy link
Copy Markdown
Collaborator

@stepanblyschak can you close and reopen? maybe it can help.

@liat-grozovik
Copy link
Copy Markdown
Collaborator

@qiluo-msft @volodymyrsamotiy please review

@qiluo-msft
Copy link
Copy Markdown
Contributor

/easycla

for npu in range(self.num_npus):
run_command(['systemctl', action, f'{name}@{npu}'])

def _enable_feature(self, package: Package, block: bool = True):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_enable_feature

This function is never used.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@qiluo-msft Removed

Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
@stepanblyschak
Copy link
Copy Markdown
Contributor Author

/azpw run

@mssonicbld
Copy link
Copy Markdown
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Copy Markdown
Collaborator

@qiluo-msft any further comment or this can be approved and merge?
@stepanblyschak is this a bug fix for master only or also for 202305?

@stepanblyschak
Copy link
Copy Markdown
Contributor Author

@liat-grozovik This change is needed for 2205, 2211 and 2305

@liat-grozovik
Copy link
Copy Markdown
Collaborator

@lguohan the urgency is to get this on master and 202305. can you please assign someone?

@bingwang-ms bingwang-ms merged commit 3610ce9 into sonic-net:master Nov 20, 2023
nmoray pushed a commit to nmoray/sonic-utilities that referenced this pull request Jun 25, 2025
…eature has constraints in YANG model on FEATURE table (sonic-net#2933)

* [sonic-package-manager] Fix YANG validation failure on upgrade when feature has constraints in YANG model on FEATURE table


Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
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.

6 participants