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.

Sign up for GitHub

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

Jump to bottom

[WIP]Ignore network reconfiguration if deployscope bootstrap #312

Closed
wants to merge 3 commits into from
Closed

[WIP]Ignore network reconfiguration if deployscope bootstrap #312

wants to merge 3 commits into from

Conversation

vineela-pachchipulusu
Copy link
Contributor

After the platformNetwork configuration of the OAM/MGMT/ADMIN networks,
there is a chance that the networks configured in deployment config yaml
is not aligned with the bootstrap values.

DM shall not overwrite the networks configured during the bootstrap.
Resource platformNetworks shall be out-of-sync and not reconciled
thereby raising an alarm for user action.

Test Plan:

  1. PASS: Deployment config yaml applied with deploymentScope: "bootstrap"
    for platformNetwork resource, insync will be false, reconcile will be false,
    and alarm will be raised.

  2. PASS: Deployment config yaml applied with deploymentScope: "principal"
    for platformNetwork resource, insync will be true, reconcile will be true,
    and config will be overwritten.

@vineela-pachchipulusu vineela-pachchipulusu changed the title [WIP]Ignore network reonfiguration if deployscope bootstrap [WIP]Ignore network reconfiguration if deployscope bootstrap Jan 11, 2024
@vineela-pachchipulusu vineela-pachchipulusu force-pushed the testing branch 2 times, most recently from f1fafc6 to d11e226 Compare January 16, 2024 14:01
@yjian118
Copy link
Collaborator

Can you rebase your branch and only cherry-pick your commit in this PR? I will only review your commit in this PR

yjian118
yjian118 reviewed Jan 16, 2024
View reviewed changes
controllers/platformnetwork_controller.go
@@ -835,7 +835,17 @@ func (r *PlatformNetworkReconciler) ReconcileResource(client *gophercloud.Servic
return err
}
if instance.Status.DeploymentScope == cloudManager.ScopeBootstrap {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We only want to ignore configuration of oam/mgmt/admin networks, should allow the other types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the code with check for network type

@hugonicodemos
Copy link
Collaborator

Can you rebase your branch and only cherry-pick your commit in this PR? I will only review your commit in this PR

+2, is hard to review it with Sriram's PR as well

@sriram-gn
Copy link
Contributor

sriram-gn commented Jan 17, 2024 •
edited
Loading 

if instance.Status.DeploymentScope == cloudManager.ScopeBootstrap {
			inSync := !update_pn
			if instance.Status.InSync != inSync {
				r.ReconcilerEventLogger.NormalEvent(instance, common.ResourceUpdated, "synchronization has changed to: %t", inSync)
			}
			instance.Status.InSync = inSync
			instance.Status.Reconciled = inSync
			err2 := r.Client.Status().Update(context.TODO(), instance)
			if err2 != nil {
				logPlatformNetwork.Error(err2, "failed to update platform network status")
			}
			return err2
		}

@yjian118 @hugonicodemos only above is the code snippet from this commit.
It makes use of update_pn and associated functions from my commit.

sriram-gn and others added 2 commits January 17, 2024 07:05
@sriram-gn @vineela-pachchipulusu
Support for Platform Network Reconfiguration
952b5fe 
This commit enables platform network reconfiguration which is a Day-2
operation.
The scope of this commit is limited to AIO-SX systems and oam, mgmt, admin
network types.
The network reconfigurtion will be ignored if the system is of other types.

Test Cases:
1. PASS - Verify network reconfig is successful on AIO-SX systems by
updating platform resource configurations with deploymentScope of
"principal" for oam/mgmt/admin networks.

2. PASS - Verify network reconfig is successful on AIO-SX systems by
applying new platform resource configurations with deploymentScope of
"principal" for oam/mgmt/admin networks.

3. PASS - Verify network reconfig is ignored on systems other than
AIO-SX despite setting deploymentScope of "principal". The values for
"INSYNC" and "RECONCILED" should reflect true reconciliation status.

4. PASS - Verify that network reconfig is ignored if deploymentScope is
"bootstrap" and the values for "INSYNC" and "RECONCILED" reflect the
true reconcialiation status.

Signed-off-by: Sriram Narasimhan <Sriram.Narasimhan@windriver.com>
(cherry picked from commit d6d0c8d)
(cherry picked from commit 8a06618)
@vineela-pachchipulusu
Ignore network reconfiguration if deployscope bootstrap
753fe06 
After the platformNetwork configuration of the OAM/MGMT/ADMIN networks,
there is a chance that the networks configured in deployment config yaml
is not aligned with the bootstrap values.

DM shall not overwrite the networks configured during the bootstrap.
Resource platformNetworks shall be out-of-sync and not reconciled
thereby raising an alarm for user action.

Test Plan:

1. PASS: Deployment config yaml applied with deploymentScope: "bootstrap"
for platformNetwork resource, insync will be false, reconcile will be false,
and alarm will be raised.

2. PASS: Deployment config yaml applied with deploymentScope: "principal"
for platformNetwork resource, insync will be true, reconcile will be true,
and config will be overwritten.

Signed-off-by: vineela-pachchipulusu <Vineela.Pachchipulusu@windriver.com>
(cherry picked from commit d11e226)
@hugonicodemos
Copy link
Collaborator

@yjian118 @hugonicodemos only above is the code snippet from this commit. It makes use of update_pn and associated functions from my commit.

let's focus on your PR then.

@vineela-pachchipulusu
Ignoring only oam/mgmt/admin network types if deploymentScope bootstrap
9a87465 
Signed-off-by: vineela-pachchipulusu <Vineela.Pachchipulusu@windriver.com>
@vineela-pachchipulusu
Copy link
Contributor Author

This PR is closed because the code is covered in the PR https://github.com/Wind-River/cloud-platform-deployment-manager/pull/315

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yjian118 yjian118 yjian118 left review comments

@hugonicodemos hugonicodemos Awaiting requested review from hugonicodemos

@sselvara1 sselvara1 Awaiting requested review from sselvara1

@sriram-gn sriram-gn Awaiting requested review from sriram-gn

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vineela-pachchipulusu @yjian118 @hugonicodemos @sriram-gn