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

[US3990]refactor:(cstor-operator): refactor cstor operator #867

Merged
merged 4 commits into from Jan 2, 2019

Conversation

sonasingh46
Copy link
Contributor

@sonasingh46 sonasingh46 commented Dec 16, 2018

This PR will do following:

  1. Iteration for pool creation will not happen at runtasks
    rather cstor-operator will one by one create pools to converge
    to maxPool count.

  2. minPool field in SPC is deprecated. Providing minPool field in
    SPC won't do any thing.

  3. Reconciliation has been added to manual provisioned pool.

Signed-off-by: sonasingh46 sonasingh46@gmail.com

This commit will do following:
1. Iteration for pool creation will not happen at runtasks
   rather cstor-operator will one by one create pools to converge
   to maxPool count.

2. minPool field in SPC is deprecated. Providing minPool field in
   SPC won't do any thing.

3. maxPool field for manual provisioning is manadatory now to
   support reconciliation for manual provisioned pool.

4. Reconciliation has been added to manual provisioned pool.

Signed-off-by: sonasingh46 <sonasingh46@gmail.com>
@codecov
Copy link

codecov bot commented Dec 17, 2018

Codecov Report

Merging #867 into master will decrease coverage by 0.14%.
The diff coverage is 61.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #867      +/-   ##
==========================================
- Coverage   42.27%   42.12%   -0.15%     
==========================================
  Files         141      141              
  Lines        9380     9275     -105     
==========================================
- Hits         3965     3907      -58     
+ Misses       5149     5103      -46     
+ Partials      266      265       -1
Impacted Files Coverage Δ
pkg/install/v1alpha1/cstor_pool.go 0% <ø> (ø) ⬆️
cmd/maya-apiserver/spc-watcher/handler.go 57.66% <50.6%> (+1.82%) ⬆️
...d/maya-apiserver/spc-watcher/storagepool_create.go 45.28% <65.62%> (-0.6%) ⬇️
cmd/maya-apiserver/spc-watcher/select_disk.go 68.88% <80%> (+0.65%) ⬆️
cmd/cstor-volume-mgmt/app/command/start.go 50% <0%> (-28.58%) ⬇️
cmd/maya-exporter/app/collector/cstorcollector.go 88.48% <0%> (-1.09%) ⬇️
cmd/cstor-volume-grpc/app/command/commands.go
cmd/cstor-volume-grpc/api/handler.go
cmd/cstor-volume-grpc/app/command/start.go
pkg/util/iscsi.go 0% <0%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb3724e...e45d35b. Read the comment docs.

Copy link

@AmitKumarDas AmitKumarDas left a comment

Choose a reason for hiding this comment

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

@sonasingh46 I have provided some comments.
Comments on name of custom resources needs to be resolved.

cmd/maya-apiserver/spc-watcher/handler.go Outdated Show resolved Hide resolved
cmd/maya-apiserver/spc-watcher/handler.go Outdated Show resolved Hide resolved
cmd/maya-apiserver/spc-watcher/handler.go Outdated Show resolved Hide resolved
cmd/maya-apiserver/spc-watcher/handler.go Outdated Show resolved Hide resolved
cmd/maya-apiserver/spc-watcher/handler.go Outdated Show resolved Hide resolved
cmd/maya-apiserver/spc-watcher/select_disk.go Show resolved Hide resolved
pkg/install/v1alpha1/cstor_pool.go Outdated Show resolved Hide resolved
pkg/install/v1alpha1/cstor_pool.go Outdated Show resolved Hide resolved
pkg/install/v1alpha1/cstor_pool.go Outdated Show resolved Hide resolved
pkg/install/v1alpha1/cstor_pool.go Outdated Show resolved Hide resolved
Copy link

@AmitKumarDas AmitKumarDas left a comment

Choose a reason for hiding this comment

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

/lgtm

Signed-off-by: sonasingh46 <sonasingh46@gmail.com>

// If current pool count is less than maxpool count, try to converge to maxpool
if currentPoolCount < int(spcGot.Spec.MaxPools) {
maxPoolCount := int(spcGot.Spec.MaxPools)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since manual pool SPC didn't have this in 0.8, test if the upgrade will cause a failure here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should not cause but I will test it.

@sonasingh46 sonasingh46 changed the title refactor:(cstor-operator): refactor cstor operator [US3990]refactor:(cstor-operator): refactor cstor operator Dec 23, 2018
Signed-off-by: sonasingh46 <sonasingh46@gmail.com>
@sonasingh46 sonasingh46 removed the WIP label Dec 24, 2018
// validate can mutate spc object -- for example if maxPool field is not present in case
// of auto provisioning, maxPool will default to 3.
// We need to immediately patch SPC object here.
if mutateSpc && !resync {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this case cause any issue with the following scenario:

  • maya-apiserver (cstor-operator) is down
  • SPC yaml is applied.

What does resync variable indicate?

Signed-off-by: sonasingh46 <sonasingh46@gmail.com>
sonasingh46 added a commit to sonasingh46/maya that referenced this pull request Jan 21, 2019
…rchive#867)

* refactor:(cstor-operator): refactor cstor operator

This commit will do following:
1. Iteration for pool creation will not happen at runtasks
   rather cstor-operator will one by one create pools to converge
   to maxPool count.

2. minPool field in SPC is deprecated. Providing minPool field in
   SPC won't do any thing.

3. maxPool field for manual provisioning is manadatory now to
   support reconciliation for manual provisioned pool.

4. Reconciliation has been added to manual provisioned pool.

Signed-off-by: sonasingh46 <sonasingh46@gmail.com>
vishnuitta pushed a commit that referenced this pull request Jan 25, 2019
* refactor:(cstor-operator): refactor cstor operator

This commit will do following:
1. Iteration for pool creation will not happen at runtasks
   rather cstor-operator will one by one create pools to converge
   to maxPool count.

2. minPool field in SPC is deprecated. Providing minPool field in
   SPC won't do any thing.

3. maxPool field for manual provisioning is manadatory now to
   support reconciliation for manual provisioned pool.

4. Reconciliation has been added to manual provisioned pool.

Signed-off-by: sonasingh46 <sonasingh46@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants