Skip to content

Added ND backup schedule module to manage backup schedule jobs on ND 4.1 and later (DCNE-496) #162

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sajagana
Copy link
Collaborator

No description provided.

aliases: [ scheduler_start_time, start_time, time ]
backup_type:
description:
- The O(backup_type=config_only) option creates a snapshot that specifically captures the configuration settings of the Nexus Dashboard.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a description for backup_type?

  • This parameter specifies the kind of snapshot created for the Nexus Dashboard.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

path = "/api/v1/infra/backups/schedules"
schedules = nd.query_obj(path)
if name:
for schedule in schedules.get("schedules"):
Copy link
Collaborator

@shrsr shrsr Jul 30, 2025

Choose a reason for hiding this comment

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

I think it would be good to use a generic function here (which I'm using in my module)

nd.existing = nd.previous = nd.get_object_by_nested_key_value(path, nested_key_path="name", value=name, data_key="schedules")
if nd.existing:
    path = "{0}/{1}".format(path, name)

This should be added in nd.py of module_utils:

def get_object_by_nested_key_value(self, path, nested_key_path, value, data_key=None):

        response_data = self.request(path, method="GET")

        if not response_data:
            return None

        object_list = []
        if isinstance(response_data, list):
            object_list = response_data
        elif data_key and data_key in response_data:
            object_list = response_data.get(data_key)
        else:
            return None

        keys = nested_key_path.split('.')

        for obj in object_list:
            current_level = obj
            for key in keys:
                if isinstance(current_level, dict):
                    current_level = current_level.get(key)
                else:
                    current_level = None
                    break

            if current_level == value:
                return obj

        return None

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added the function to nd.py and changed the function return value based on the input value.

nd.existing = schedules

if state == "present":
if nd.existing and nd.existing.get("name") == name:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we simplify this as shown below if applicable?

payload = {
    "encryptionKey": encryption_key,
    "name": name,
    "type": backup_type,
    "frequency": frequency,
    "remoteLocation": remote_location,
    "startTime": start_date_time,
}

if nd.existing and nd.existing.get("name") == name:
    payload["frequency"] = frequency or nd.existing.get("frequency")
    payload["remoteLocation"] = remote_location or nd.existing.get("remoteLocation")
    payload["startTime"] = start_date_time or nd.existing.get("startTime")

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@sajagana sajagana requested a review from shrsr July 30, 2025 17:27
path = "{0}/{1}".format(path, name)
break

schedules = nd.get_object_by_nested_key_value(path, "name", name, data_key="schedules")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This only returns the schedule with the name

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

        if not value and response_data and data_key and data_key in response_data:
            return response_data.get(data_key)
        elif not response_data:
            return None

I changed the suggested code to support query one and query all objects based on the input value. Should I revert this change?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think it's best to let the function do what its name suggests

@sajagana sajagana requested a review from shrsr July 31, 2025 04:09
path = "{0}/{1}".format(path, name)
break

schedules = nd.get_object_by_nested_key_value(path, "name", name, data_key="schedules")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I think it's best to let the function do what its name suggests

type: str
encryption_key:
description:
- The encryption_key for a backup file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- The encryption_key for a backup file.
- The encryption key for a backup file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

---
module: nd_backup_schedule
version_added: "0.5.0"
short_description: Manages backup schedule on Cisco Nexus Dashboard.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
short_description: Manages backup schedule on Cisco Nexus Dashboard.
short_description: Manages backup schedules on Cisco Nexus Dashboard.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

version_added: "0.5.0"
short_description: Manages backup schedule on Cisco Nexus Dashboard.
description:
- Manage backup schedule on Cisco Nexus Dashboard.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Manage backup schedule on Cisco Nexus Dashboard.
- Manage backup schedules on Cisco Nexus Dashboard.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@sajagana sajagana requested review from shrsr and samiib August 1, 2025 07:29
Copy link
Collaborator

@shrsr shrsr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@samiib samiib left a comment

Choose a reason for hiding this comment

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

LGTM

- cm_rm_backup_schedule.previous.name == "backupschedule1"
- cm_rm_backup_schedule.previous.remoteLocation == "test"
- cm_rm_backup_schedule.previous.startTime == "2025-01-02T15:04:05Z"
- cm_rm_backup_schedule.previous.type == "configOnly"
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we not add a test case with backup_type parameter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

Copy link
Collaborator

@shrsr shrsr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@samiib samiib left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@anvitha-jain anvitha-jain left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

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

LGTM

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.

5 participants