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

Remove Azure location requirement #344

Merged
merged 1 commit into from
Mar 5, 2018

Conversation

ncdc
Copy link
Contributor

@ncdc ncdc commented Mar 1, 2018

Instead of requiring the Ark admin to specify a "location" in the azure
persistentVolumeProvider config (meaning only a single location is
supported), get info about the disk (for its location) when creating a
snapshot, and get info about the snapshot (for its location) when
creating a disk from a snapshot.

Signed-off-by: Andy Goldstein andy.goldstein@gmail.com

@nrb
Copy link
Contributor

nrb commented Mar 5, 2018

This appears to have worked for me, but I'd like a 2nd look as it appears I also had some Azure permissions issues. I don't believe they're related, but want to be sure.

Copy link
Contributor

@skriss skriss left a comment

Choose a reason for hiding this comment

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

One nit. Otherwise LGTM. I ran through a full test using our nginx-example on AKS in canadacentral - looks good!

@@ -100,7 +100,6 @@ No parameters required.

| Key | Type | Default | Meaning |
| --- | --- | --- | --- |
| `location` | string | Required Field | *Example*: "Canada East"<br><br>See [the list of available locations][5] (note that this particular page refers to them as "Regions"). |
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove link [5] below.

Instead of requiring the Ark admin to specify a "location" in the azure
persistentVolumeProvider config (meaning only a single location is
supported), get info about the disk (for its location) when creating a
snapshot, and get info about the snapshot (for its location) when
creating a disk from a snapshot.

Signed-off-by: Andy Goldstein <andy.goldstein@gmail.com>
@ncdc ncdc force-pushed the remove-azure-location-requirement branch from 6389a71 to 331e0c2 Compare March 5, 2018 20:21
@ncdc
Copy link
Contributor Author

ncdc commented Mar 5, 2018

@skriss fixed and rebased on top of latest master

@skriss
Copy link
Contributor

skriss commented Mar 5, 2018

LGTM

@skriss skriss added this to the v0.8.0 milestone Mar 5, 2018
@skriss skriss merged commit 973f630 into vmware-tanzu:master Mar 5, 2018
@ncdc ncdc deleted the remove-azure-location-requirement branch May 23, 2018 16:15
weshayutin pushed a commit to weshayutin/velero that referenced this pull request Jan 15, 2025
This fixes the PR vmware-tanzu#334 where one additional line was
in the code. This was not exposed previously as we
did not had downstream CI Lint jobs.

Signed-off-by: Michal Pryc <mpryc@redhat.com>
Co-authored-by: Michal Pryc <mpryc@redhat.com>
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.

3 participants