-
Notifications
You must be signed in to change notification settings - Fork 961
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 jindo runtime with replicas/NodeAffinity #493
Conversation
@@ -30,24 +40,100 @@ func (e *JindoEngine) CreateVolume() (err error) { | |||
|
|||
// createFusePersistentVolume | |||
func (e *JindoEngine) createFusePersistentVolume() (err 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.
suggest adding a pkg named render
for rendering kube native resource
convenient for reuse
default: | ||
cond = utils.NewDatasetCondition(datav1alpha1.DatasetReady, datav1alpha1.DatasetReadyReason, | ||
"The ddc runtime is unknown.", | ||
corev1.ConditionFalse) |
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.
corev1.ConditionUnknown
?
return err | ||
} | ||
|
||
return | ||
} | ||
|
||
func (e *JindoEngine) UpdateCacheOfDataset() (err 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.
je
is better ? receiver-names
pkg/ddc/jindo/delete_volume.go
Outdated
if err != nil { | ||
return err | ||
} | ||
retries := 500 |
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.
hardcode ?
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.
fix
pkg/ddc/jindo/cache.go
Outdated
|
||
// queryCacheStatus checks the cache status | ||
func (e *JindoEngine) queryCacheStatus() (states cacheStates, err error) { | ||
/*summary, err := e.reportSummary() |
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 the code if it is not used ?
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.
will implement soon
ports: | ||
- containerPort: 8080 | ||
name: metrics | ||
protocol: TCP | ||
resources: |
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.
为什么资源限制移除了?
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.
fix
|
||
if (e.getTieredStoreType(runtime) == 0) { | ||
// MEM | ||
properties["storage.ram.cache.size"] = utils.TranformQuantityToJindoUnit(runtime.Spec.Tieredstore.Levels[0].Quota) |
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.
need make sure the length of runtime.Spec.Tieredstore.Levels
is greater than 0, otherwise cause panic
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.
fix
func TranformQuantityToJindoUnit(q *resource.Quantity) (value string) { | ||
value = q.String() | ||
if strings.HasSuffix(value, "Gi") { | ||
value = strings.ReplaceAll(value, "Gi", "g") |
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.
can JindoUnit follow the the same standard ?
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.
can not be same with alluxio now
@@ -44,7 +44,7 @@ spec: | |||
args: ["worker"] | |||
env: | |||
- name: STORAGE_NAMESPACE_RPC_ADDRESS | |||
value: {{ template "jindofs.fullname" . }}-master:{{ .Values.master.namespace.rpc.port }} |
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.
建议端口动态指定哈。
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.
下个PR处理
e.getMountPoint(), | ||
common.JINDO_MOUNT_TYPE, | ||
e.Log) | ||
if !found { |
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.
为什么没有使用通用实现呢?
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.
fluid/pkg/utils/dataset/volume/create.go
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.
fix
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.
已使用通用实现
@@ -21,7 +37,7 @@ func (r *RuntimeReconciler) getRuntime(ctx cruntime.ReconcileRequestContext) (*d | |||
return &runtime, nil | |||
} | |||
|
|||
// GetOrCreateEngine gets the engine | |||
// GetOrCreateEngine gets the dataset |
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.
why change engine to dataset?
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.
fix
} | ||
|
||
// createFusePersistentVolumeClaim | ||
// createFusePersistentVolume |
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.
why change createFusePersistentVolumeClaim to createFusePersistentVolume?
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.
fix
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.
comment change
) | ||
|
||
func (e *JindoEngine) UpdateDatasetStatus(phase datav1alpha1.DatasetPhase) (err error) { | ||
/*err = retry.RetryOnConflict(retry.DefaultBackoff, func() error { | ||
// 1. update the runtime status |
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.
- get the runtime status
pkg/ddc/jindo/delete_volume.go
Outdated
volumeHelper "github.com/fluid-cloudnative/fluid/pkg/utils/dataset/volume" | ||
) | ||
|
||
// DeleteVolume creates volume |
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.
DeleteVolume delete Fuse PersistentVolumeClaim and PersistentVolume
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.
fix
Codecov Report
@@ Coverage Diff @@
## master #493 +/- ##
==========================================
- Coverage 19.72% 19.68% -0.04%
==========================================
Files 57 57
Lines 2119 2123 +4
==========================================
Hits 418 418
- Misses 1656 1660 +4
Partials 45 45
Continue to review full report at Codecov.
|
Ⅰ. Describe what this PR does
add jindo runtime with replicas/NodeAffinity
add some jindo opreation with mock
Ⅱ. Does this pull request fix one issue?
fixes #495
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews