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

Migrate AAA table in db_migrator #3284

Merged
merged 13 commits into from
Apr 29, 2024

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Apr 23, 2024

Migrate AAA table in db_migrator

Why I did it

per-command AAA need enable in warm-upgrade case

How I did it

Add db_migrator code to migrate AAA table

How to verify it

Pass all test case.
Add new test case.

Which release branch to backport (provide reason below if selected)

N/A

Description for the changelog

Migrate AAA table in db_migrator

A picture of a cute animal (not mandatory but encouraged)

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-utilities

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80 liuh-80 marked this pull request as ready for review April 24, 2024 08:36
@liuh-80 liuh-80 requested a review from qiluo-msft April 24, 2024 08:36
@qiluo-msft qiluo-msft requested a review from vaibhavhd April 27, 2024 00:49
return

authentication_config_old = self.configDB.get_entry('AAA', 'authentication')
# If device enabled TACACS authentication, then enable TACACS per-command accounting and authorization
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 27, 2024

Choose a reason for hiding this comment

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

enable

The decision should also be driven by GOLDEN_CFG_FILE. GOLDEN_CFG_FILE could express the intention to enable AAA feature or disable, and db_migrator should respect the intention. ref: migrate_telemetry() #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

The old image's enablement does not matter. As long as the required passkey is there, and intention is to enable, then we should enable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, also discussion with Gang about how to handle version, because AAA table should migrate on all version >= 202205, so add migrate to common_migration_ops.

return

authentication_config_old = self.configDB.get_entry('AAA', 'authentication')
# If device enabled TACACS authentication, then enable TACACS per-command accounting and authorization
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 27, 2024

Choose a reason for hiding this comment

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

accounting

Is it better to use separate PRs for accounting and authorization enablement? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, this PR only contains migrate accounting part. will submit another PR later.

@liuh-80 liuh-80 changed the title Enable per-command authorization in db_migrator Migrate AAA table in db_migrator Apr 29, 2024
@liuh-80 liuh-80 requested a review from ganglyu April 29, 2024 05:37
ganglyu
ganglyu previously approved these changes Apr 29, 2024
qiluo-msft
qiluo-msft previously approved these changes Apr 29, 2024
@liuh-80 liuh-80 dismissed stale reviews from qiluo-msft and ganglyu via 3f3dcbf April 29, 2024 06:51
@liuh-80 liuh-80 merged commit 4c04013 into sonic-net:master Apr 29, 2024
5 checks passed
@liuh-80
Copy link
Contributor Author

liuh-80 commented Apr 29, 2024

Per-command authorization migrate code add in this PR: #3296

liuh-80 added a commit to liuh-80/sonic-utilities that referenced this pull request May 20, 2024
Migrate AAA table in db_migrator

    per-command AAA need enable in warm-upgrade case

    Add db_migrator code to migrate AAA table

    Pass all test case.
    Add new test case.

    N/A

    Migrate AAA table in db_migrator
mssonicbld pushed a commit to mssonicbld/sonic-utilities that referenced this pull request May 21, 2024
Migrate AAA table in db_migrator

#### Why I did it
    per-command AAA need enable in warm-upgrade case

#### How I did it
    Add db_migrator code to migrate AAA table

#### How to verify it
    Pass all test case.
    Add new test case.

#### Which release branch to backport (provide reason below if selected)
    N/A

#### Description for the changelog
    Migrate AAA table in db_migrator

#### A picture of a cute animal (not mandatory but encouraged)
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #3329

mssonicbld pushed a commit that referenced this pull request May 21, 2024
Migrate AAA table in db_migrator

#### Why I did it
    per-command AAA need enable in warm-upgrade case

#### How I did it
    Add db_migrator code to migrate AAA table

#### How to verify it
    Pass all test case.
    Add new test case.

#### Which release branch to backport (provide reason below if selected)
    N/A

#### Description for the changelog
    Migrate AAA table in db_migrator

#### A picture of a cute animal (not mandatory but encouraged)
arfeigin pushed a commit to arfeigin/sonic-utilities that referenced this pull request Jun 16, 2024
Migrate AAA table in db_migrator

#### Why I did it
    per-command AAA need enable in warm-upgrade case

#### How I did it
    Add db_migrator code to migrate AAA table

#### How to verify it
    Pass all test case.
    Add new test case.

#### Which release branch to backport (provide reason below if selected)
    N/A

#### Description for the changelog
    Migrate AAA table in db_migrator

#### A picture of a cute animal (not mandatory but encouraged)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants