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

CSM Replication Support For PowerScale #18

Merged
merged 6 commits into from
Jan 31, 2022
Merged

Conversation

IliaRN
Copy link
Contributor

@IliaRN IliaRN commented Jan 26, 2022

PR Submission checklist

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#116

Common PR Checklist:

  • Have you made sure that the code compiles?
  • Have you commented your code, particularly in hard-to-understand areas
  • Did you run tests in a real Kubernetes cluster?
  • Have you maintained backward compatibility

Description of your changes:

This PR addresses an issue with CSM Replication Support For PowerScale. Added initial replication code and DeletePolicy method.

p := &Policies{}
err = client.Get(ctx, policiesPath, name, nil, nil, &p)
if err != nil || len(p.Policy) == 0 {
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

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

In a scenario where err is nil and len(p.Policy)==0 , we will be sending nil, nil essentially.
May be we can consider creating an err and returning it.

@randeepdell
Copy link
Contributor

Need to rebase again, this PR shows changes that are already merged.

@IliaRN IliaRN force-pushed the feature/replication branch from e1a5804 to 2bec035 Compare January 28, 2022 10:37
@IliaRN IliaRN force-pushed the feature/replication branch from 2bec035 to 67f809d Compare January 28, 2022 11:43
@prablr79
Copy link
Collaborator

@IliaRN can you check few comments ? post addressing that we can merge this PR.

Copy link
Contributor

@randeepdell randeepdell left a comment

Choose a reason for hiding this comment

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

LGTM

@prablr79
Copy link
Collaborator

@randeepsharma is rebase is done for this PR ?

@randeepdell
Copy link
Contributor

@randeepsharma is rebase is done for this PR ?

Yes

@bpjain2004 bpjain2004 merged commit bf7e8c0 into main Jan 31, 2022
@bpjain2004 bpjain2004 deleted the feature/replication branch January 31, 2022 06:25
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.

6 participants