Skip to content

Conversation

@TrafalgarZZZ
Copy link
Member

@TrafalgarZZZ TrafalgarZZZ commented Dec 28, 2023

Ⅰ. Describe what this PR does

Fluid has supported more and more runtimes and some of the runtimes also have different underlying implementations. Such implementations are supported in code with different Engine. Before all these concepts turn into a mess-up, we have to clearly define the relationship between Engines and Runtimes and all the related concepts in the code.

This PR does the following changes:

  • Add a new field called engineImpl, specifying the underlying engine implementation. engineImpl has a one-to-one relation with a concrete Engine.
    • e.g. JindoCacheEngine means engineImpl=="jindocache"
  • runtimeType now only refers to what the runtime is so it has a one-to-one relation with a specific runtime CRD.
    • e.g. AlluxioRuntime means runtimeType=="alluxio"
  • Each Runtime may have several engine implementations, so runtimeType has a one-to-many relation with engineImpl.
    • e.g. JindoRuntime now has three engine implementations: JindoFS、JindoFSx、JindoCache

Other minor changes:

  • Some typo fixes
  • Renamed some funcs for better readability

IMPORTANT:

  • Engine impl auto-infer is not supported for ThinRuntime because ReferenceDatasetEngine is now an exception. Should we consider ReferenceDatasetEngine as an engine impl?
  • StorageLabel also relies on runtimeType but JindoRuntime may suffer from breaking change after the PR merges: some storage labels may not be cleaned up when the runtime is shut down.

Ⅱ. Does this pull request fix one issue?

Resolves #3673

Ⅲ. 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

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
…DatasetEngine

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
…orts`

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
@codecov
Copy link

codecov bot commented Dec 28, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (3351f64) 64.31% compared to head (3058a4d) 64.40%.
Report is 2 commits behind head on master.

Files Patch % Lines
pkg/ddc/thin/master_internal.go 40.00% 3 Missing ⚠️
pkg/ddc/base/operation_helm.go 0.00% 1 Missing ⚠️
pkg/ddc/efc/shutdown.go 66.66% 1 Missing ⚠️
pkg/ddc/jindo/master.go 0.00% 1 Missing ⚠️
pkg/ddc/jindocache/master.go 0.00% 1 Missing ⚠️
pkg/ddc/jindofsx/master.go 0.00% 1 Missing ⚠️
pkg/ddc/juicefs/shutdown.go 50.00% 1 Missing ⚠️
pkg/ddc/thin/master.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3672      +/-   ##
==========================================
+ Coverage   64.31%   64.40%   +0.08%     
==========================================
  Files         444      444              
  Lines       26780    26797      +17     
==========================================
+ Hits        17223    17258      +35     
+ Misses       7542     7524      -18     
  Partials     2015     2015              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cheyang
Copy link
Collaborator

cheyang commented Dec 28, 2023

/test fluid-e2e

Copy link
Collaborator

@cheyang cheyang left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@TrafalgarZZZ
Copy link
Member Author

/retest

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
… compatibility

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
@fluid-e2e-bot fluid-e2e-bot bot removed the lgtm label Dec 29, 2023
@sonarqubecloud
Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

7 New issues
0 Security Hotspots
No data about Coverage
8.3% Duplication on New Code

See analysis details on SonarCloud

@TrafalgarZZZ
Copy link
Member Author

/test fluid-e2e

1 similar comment
@TrafalgarZZZ
Copy link
Member Author

/test fluid-e2e

Copy link
Collaborator

@cheyang cheyang left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@fluid-e2e-bot fluid-e2e-bot bot added the lgtm label Dec 29, 2023
@fluid-e2e-bot
Copy link

fluid-e2e-bot bot commented Dec 29, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheyang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fluid-e2e-bot fluid-e2e-bot bot merged commit efb0ae4 into fluid-cloudnative:master Dec 29, 2023
xliuqq pushed a commit to xliuqq/fluid that referenced this pull request Jan 9, 2024
…uid-cloudnative#3672)

* Add missing valueFile property in JuiceFSRuntime's status when syncing

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Add missing valueFile property in ThinRuntime's status when syncing

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Add `engineImpl` for specifying runtime's backended engines

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Change runtimeType to engineImpl for JindoFS Engine

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Change runtimeType to engineImpl for JindoFSx Engine

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Change runtimeType to engineImpl for JindoCache Engine

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Change runtimeType to engineImpl for Alluxio Engine

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Change runtimeType to engineImpl for JuiceFS Engine

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Change runtimeType to engineImpl for GooseFS Engine

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Fix unit tests

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Change runtimeType to engineImpl for Thin Engine

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Change runtimeType to engineImpl for EFC Engine

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Fix thinruntime not to use ddc.InferEngineImpl() because of ReferenceDatasetEngine

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Auto-infer engine implementation for Operation Reconciler

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Change port allocator port reservation func to `jindofsx.GetReservedPorts`

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Fix Data Operation helm installation mistake

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

* Hack engine.getRuntimeInfo's runtimeType with engineImpl for backward compatibility

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>

---------

Signed-off-by: trafalgarzzz <trafalgarz@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURES] Automatically infer engine implementation to support multi-engine runtimes

2 participants