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

Added Support to render Feature Table using Device running metadata. #14

Merged
merged 6 commits into from
Sep 22, 2022

Conversation

abdosi
Copy link
Contributor

@abdosi abdosi commented Aug 19, 2022

What I did:
Added Support to render Feature Table using Device running metadata
Also added support to render 'has_asic_scope' field of Feature Table.

Why I did:
Details: sonic-net/sonic-buildimage#11796

How I verify:
Manual Verification
UT added including multi-asic,

Also added support to render 'has_asic_scope' field of Feature Table.

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@abdosi
Copy link
Contributor Author

abdosi commented Aug 19, 2022

cc @arlakshm @judyjoseph

@abdosi
Copy link
Contributor Author

abdosi commented Aug 19, 2022

cc @anamehra @SuvarnaMeenakshi

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@abdosi
Copy link
Contributor Author

abdosi commented Sep 1, 2022

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 14 in repo sonic-net/sonic-host-services

@abdosi
Copy link
Contributor Author

abdosi commented Sep 1, 2022

/azp run sonic-net.sonic-host-services

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 14 in repo sonic-net/sonic-host-services

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@abdosi
Copy link
Contributor Author

abdosi commented Sep 2, 2022

@arlakshm can you please help review this.

@arlakshm
Copy link
Contributor

arlakshm commented Sep 9, 2022

@abdosi , can you update the branch ?

.format(feature.name, feature_suffixes[-1]))
self.set_feature_state(feature, self.FEATURE_STATE_FAILED)
return
self._config_db.mod_entry('FEATURE', feature_config.name, {'has_per_asic_scope': str(feature_config.has_per_asic_scope)})
Copy link
Contributor

Choose a reason for hiding this comment

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

The host config_db will have has_per_asic_scope as failed but the config_db for the asic will have them as enabled? should we update that as well to avoid confusion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arlakshm yes, that we can do later if needed. Currently also other fields of this table are not same between host and asic.
I think that change will be coming as part of separate PR may be where we needed to update (ex:macsec feature state) that need be updated in asic db also. In that PR we can update this field also.

@judyjoseph

@mlok-nokia
Copy link
Contributor

mlok-nokia commented Sep 20, 2022

@arlakshm & @abdosi Hi, Is this PR going be merged soon? We need this fix to check what else we need to do to address some of Error messages on Supervisor card for some of the feature such lldp0,lldp1...

yes this should get merge soon.

@abdosi
Copy link
Contributor Author

abdosi commented Sep 20, 2022

@abdosi , can you update the branch ?

@arlakshm done

@abdosi abdosi merged commit 6e45acc into sonic-net:master Sep 22, 2022
@abdosi abdosi deleted the feature branch September 22, 2022 00:18
abdosi added a commit to abdosi/sonic-buildimage that referenced this pull request Nov 6, 2022
into 202205 branch

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
abdosi added a commit to sonic-net/sonic-buildimage that referenced this pull request Nov 8, 2022
ganglyu pushed a commit that referenced this pull request Feb 13, 2023
Cherry-pick of #14 in 202205 branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants