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

Update OAM Network using oam API #305

Closed

Conversation

hugonicodemos
Copy link
Collaborator

This commits enable the OAM network using the
oam network API instead of the addresspool API.

Test Plan:

  1. After system installation setup the platformNetwork with new values of the OAM Network.
  2. Apply the deployment configuration
  3. Check the OAM Network was updated
  • system oam-show
  • system addrpool-list

This commits enable the OAM network using the
oam network API instead of the addresspool API.

Test Plan:
1. After system installation setup the platformNetwork
with new values of the OAM Network.
2. Apply the deployment configuration
3. Check the OAM Network was updated
- system oam-show
- system addrpool-list

Signed-off-by: Hugo Brito <hugo.brito@windriver.com>
@yjian118
Copy link
Collaborator

yjian118 commented Dec 7, 2023

The code lgtm. But there's a request: if the deployScope = "bootstrap", ignore the difference even if the OAM network in the deployment-config.yaml is different than the current config(should be configured by the bootstrap playbook), will it be delivered in another commit?

@@ -317,6 +414,18 @@ func (r *PlatformNetworkReconciler) ReconcileAddressPool(client *gophercloud.Ser
} else {
if pool == nil {
pool, err = r.ReconcileNewAddressPool(client, instance)
} else if instance.Spec.Type == "oam" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Personally, I prefer to use Constant for "oam" but not mandatory

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We'll handle this in a follow-up commit when we create and manage the other networks (admin, mgmt). Is that okay?

@yjian118
Copy link
Collaborator

Request a test of a lab deployment before merging this code. @hugonicodemos can you add two cases:
1 deploy sm-1
2 generate deploy-config.yaml with deployctl

We can separate the scope work items in the next commit if the sanity passed

@sselvara1
Copy link
Collaborator

sselvara1 commented Jan 9, 2024

This commits enable the OAM network using the oam network API instead of the addresspool API.

Test Plan:

  1. After system installation setup the platformNetwork with new values of the OAM Network.
  2. Apply the deployment configuration
  3. Check the OAM Network was updated
  • system oam-show
  • system addrpool-list
  1. What is the advantage of oam network API over the addresspool API for updating OAM network ?
  2. Test plan indicates day-2 operation, right?

@sselvara1
Copy link
Collaborator

sselvara1 commented Jan 9, 2024

The code lgtm. But there's a request: if the deployScope = "bootstrap", ignore the difference even if the OAM network in the deployment-config.yaml is different than the current config(should be configured by the bootstrap playbook), will it be delivered in another commit?

Request a test case for deployScope="bootstrap" and deployScope="principal" and
also capture the status for "insync" and "reconciled" in the test case.

@sselvara1
Copy link
Collaborator

Request a test of a lab deployment before merging this code. @hugonicodemos can you add two cases: 1 deploy sm-1 2 generate deploy-config.yaml with deployctl

We can separate the scope work items in the next commit if the sanity passed

As a follow up, edit deploy-config.yaml with OAM NW change and apply again (day-2)

@hugonicodemos
Copy link
Collaborator Author

New tests are performed:
New deployment-config with platformNetwork OAM changes - day1 operation (deploymentScope = bootstrap):

  • changes not allowed
  • insync = false
    NAME TYPE SUBNET PREFIX INSYNC RECONCILED SCOPE
    platformnetwork.starlingx.windriver.com/oam oam 10.10.20.0 24 false true bootstrap

@hugonicodemos
Copy link
Collaborator Author

hugonicodemos commented Jan 10, 2024

This commits enable the OAM network using the oam network API instead of the addresspool API.
Test Plan:

  1. After system installation setup the platformNetwork with new values of the OAM Network.
  2. Apply the deployment configuration
  3. Check the OAM Network was updated
  • system oam-show
  • system addrpool-list
  1. What is the advantage of oam network API over the addresspool API for updating OAM network ?
    (nicodemos) we cant change the OAM network via addresspool API.
  2. Test plan indicates day-2 operation, right?
    (nicodemos) additional test for day1 added.

@hugonicodemos
Copy link
Collaborator Author

The code lgtm. But there's a request: if the deployScope = "bootstrap", ignore the difference even if the OAM network in the deployment-config.yaml is different than the current config(should be configured by the bootstrap playbook), will it be delivered in another commit?

Request a test case for deployScope="bootstrap" and deployScope="principal" and also capture the status for "insync" and "reconciled" in the test case.

done

@@ -200,6 +263,39 @@ func (r *PlatformNetworkReconciler) ReconcileNewAddressPool(client *gophercloud.
// ReconcileUpdated is a method which handles reconciling an existing data
Copy link
Collaborator

@yjian118 yjian118 Jan 10, 2024

Choose a reason for hiding this comment

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

The description need to be allocated at the right function, expecting description to the new function as well

@yjian118
Copy link
Collaborator

I did the test as follows:
1 bootstrap a subcloud w/o the --deploy-config from its oam address 10.10.10.32
2 update its deployment-config with:

---
apiVersion: starlingx.windriver.com/v1
kind: PlatformNetwork
metadata:
  labels:
    controller-tools.k8s.io: "1.0"
  name: oam
  namespace: deployment
spec:
  allocation:
    order: random
    ranges:
    - end: 10.10.10.254
      start: 10.10.10.1
  floatingAddress: 10.10.10.42
  gateway: 10.10.10.1
  prefix: 24
  subnet: 10.10.10.0
  type: oam

3 config the subcloud with "dcmanager subcloud deploy config --deploy-config <>"

I can see the log in the DM log as:

2024-01-11T19:00:42.938426301Z  INFO    controller.platformnetwork.deployment/oam       delta configuration:
        +Floating Address: 10.10.10.42

2024-01-11T19:00:42.949473508Z  INFO    controller.platformnetwork.deployment/oam       updating oam network    {"uuid": "cea4fbdb-8528-49fd-bce9-a0859fa539a5", "opts": {"oam_floating_ip":"10.10.10.42"}}

After the deployment, my OAM ssh lost connection after unlock. And the deploy playbook failed as:

TASK [Waiting for port to become open] *****************************************
Thursday 11 January 2024  19:02:15 +0000 (0:00:38.660)       0:02:23.018 ******
fatal: [subcloud3]: FAILED! => changed=false
  elapsed: 400
  msg: Timeout when waiting for 10.10.10.32:22

I can login to the subcloud via 10.10.10.42 after the unlock:

4: enp2s1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP group default qlen 1000
    link/ether 52:54:00:2a:91:ff brd ff:ff:ff:ff:ff:ff
    inet 10.10.10.42/24 brd 10.10.10.255 scope global enp2s1
       valid_lft forever preferred_lft forever
    inet6 fe80::5054:ff:fe2a:91ff/64 scope link
       valid_lft forever preferred_lft forever

Apparently, this is a failed case from the misconfiguration of the deployment-config.yaml, but we want to prevent it if possible.
@hugonicodemos do you want to address it in this commit or I can accept a follow up commit

@hugonicodemos hugonicodemos deleted the update-oam-network branch January 18, 2024 13:58
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.

4 participants