-
Notifications
You must be signed in to change notification settings - Fork 314
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
Add CRUD support for availability zone #1049
base: development
Are you sure you want to change the base?
Conversation
f47bf25
to
e13ab5d
Compare
Codecov Report
@@ Coverage Diff @@
## development #1049 +/- ##
==============================================
+ Coverage 34.58% 34.69% +0.1%
==============================================
Files 97 100 +3
Lines 17849 18165 +316
==============================================
+ Hits 6173 6302 +129
- Misses 10804 10967 +163
- Partials 872 896 +24
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- please use AvailabilityZon instead of Zon.
- it's will have some impact of pool dicovery. When discovering pool, Dock daemon will check whether its availability zone exists in the system.
pkg/api/controllers/pool_test.go
Outdated
// assertTestResult(t, w.Code, 200) | ||
// assertTestResult(t, output, SampleAvailabilityZones) | ||
// }) | ||
// } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove unused code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
// "strings" | ||
|
||
// log "github.com/golang/glog" | ||
"github.com/opensds/opensds/pkg/api/policy" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
"github.com/opensds/opensds/pkg/model" | ||
// "github.com/opensds/opensds/pkg/utils/constants" | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
// An OpenSDS zone is identified by a unique name and ID. | ||
type ZoneSpec struct { | ||
*BaseModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use AvailabilityZoneSpec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
aaedd1f
to
189aa20
Compare
189aa20
to
baf8708
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Url: urls.GenerateZoneURL(urls.Etcd, "", zoneID), | ||
} | ||
dbRes := c.Get(dbReq) | ||
if dbRes.Status != "Success" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what will be the behaviour if c.Get() returns null or errornous code?
} | ||
dbRes := c.Get(dbReq) | ||
if dbRes.Status != "Success" { | ||
log.Error("When get zone in db:", dbRes.Error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
message should start with lower case.
please also change the below line nos with lower case
line no: 1348, 1385, 1418, 1431, 1441
azExists := false | ||
azs, err := dr.c.ListAvailabilityZones(ctx) | ||
if err != nil { | ||
log.Errorf("When list zone %s in db: %v\n", pol.AvailabilityZone, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here. And please take a look of all files about lower case starting letter
What this PR does / why we need it:
This PR adds Create/Update/List/Delete operations on availability zone so that management of zone is more flexible.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #1040Special notes for your reviewer: Implementation is in progress
Release note: