Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions pkg/ddc/alluxio/load_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -270,7 +271,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -370,7 +372,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -441,7 +444,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down
12 changes: 8 additions & 4 deletions pkg/ddc/jindo/load_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -307,7 +308,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -423,7 +425,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -510,7 +513,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down
12 changes: 8 additions & 4 deletions pkg/ddc/jindocache/load_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -304,7 +305,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -420,7 +422,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -507,7 +510,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down
12 changes: 8 additions & 4 deletions pkg/ddc/jindofsx/load_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -304,7 +305,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -420,7 +422,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -507,7 +510,8 @@ func Test_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down
12 changes: 8 additions & 4 deletions pkg/ddc/juicefs/data_load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,8 @@ func TestJuiceFSEngine_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -674,7 +675,8 @@ func TestJuiceFSEngine_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -808,7 +810,8 @@ func TestJuiceFSEngine_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down Expand Up @@ -913,7 +916,8 @@ func TestJuiceFSEngine_genDataLoadValue(t *testing.T) {
Name: "test-dataload",
OwnerDatasetId: "fluid-test-dataset",
Owner: &common.OwnerReference{
APIVersion: "/",
Kind: "DataLoad",
APIVersion: "data.fluid.io/v1alpha1",
Enabled: true,
Name: "test-dataload",
BlockOwnerDeletion: false,
Expand Down
37 changes: 35 additions & 2 deletions pkg/utils/transformer/owner_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,49 @@ limitations under the License.
package transformer

import (
datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1"
"github.com/fluid-cloudnative/fluid/pkg/common"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
)

var log = ctrl.Log.WithName("utils.transformer")

// fluidScheme knows the fluid API types. It recovers the GroupVersionKind of an object whose TypeMeta is not
// fully populated, which a typed client is allowed to hand back: an ownerReference missing its kind or its
// apiVersion is rejected by the API server and cannot be resolved back to its owner by an owner-based watch.
var fluidScheme = runtime.NewScheme()

func init() {
utilruntime.Must(datav1alpha1.AddToScheme(fluidScheme))
}

func GenerateOwnerReferenceFromObject(obj client.Object) *common.OwnerReference {
// The kind and the apiVersion fall back on their own, because a partially populated TypeMeta produces a
// reference which is just as malformed as an entirely empty one.
gvk := obj.GetObjectKind().GroupVersionKind()
if len(gvk.Kind) == 0 || len(gvk.Version) == 0 {
resolved, err := apiutil.GVKForObject(obj, fluidScheme)
if err != nil {
log.Error(err, "failed to recover the GroupVersionKind of the owner from the scheme, the generated ownerReference stays incomplete",
"namespace", obj.GetNamespace(), "name", obj.GetName(), "groupVersionKind", gvk.String())
} else {
if len(gvk.Kind) == 0 {
gvk.Kind = resolved.Kind
}
if len(gvk.Version) == 0 {
gvk.Group, gvk.Version = resolved.Group, resolved.Version
}
}
}
Comment on lines +35 to +58

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The premise here does not hold, so no change on this point — but let me show the checks rather than just assert it:

  1. Other fluid API versions: there is only one. ls api/v1alpha1, and grep -rn v1beta1 api/ returns nothing. The data.fluid.io/v1beta1 in the PR description comes from an existing test fixture that sets that string in TypeMeta by hand; that path never reaches the scheme lookup, because a populated field is used as-is. So datav1alpha1.AddToScheme covers every fluid version that exists.
  2. Non-fluid owners: all 16 call sites pass fluid custom resources (dataload, dataMigrate, dataProcess, and the various runtime types) — no built-in type is ever passed. Adding the client-go scheme would register types that no caller can reach.

Rather than widen the scheme speculatively, the failure is now observable: an unregistered type is logged (see the other thread), and a spec pins that behaviour. If a non-fluid owner ever shows up, the log will say so instead of silently producing an empty kind.


ref := &common.OwnerReference{
APIVersion: obj.GetObjectKind().GroupVersionKind().GroupKind().Group + "/" + obj.GetObjectKind().GroupVersionKind().Version,
Kind: obj.GetObjectKind().GroupVersionKind().Kind,
APIVersion: gvk.GroupVersion().String(),
Kind: gvk.Kind,
UID: string(obj.GetUID()),
Enabled: true,
Name: obj.GetName(),
Expand Down
63 changes: 63 additions & 0 deletions pkg/utils/transformer/owner_reference_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)

var _ = Describe("GenerateOwnerReferenceFromObject", func() {
Expand Down Expand Up @@ -189,6 +191,67 @@ var _ = Describe("GenerateOwnerReferenceFromObject", func() {
},
),
)

// A typed client is allowed to hand back objects whose TypeMeta is not fully populated, so the kind and
// the apiVersion have to be recovered from the scheme. Otherwise the ownerReference is rejected by the
// API server and cannot be resolved back to its owner by an owner-based watch.
DescribeTable("when the TypeMeta of the object is incomplete",
func(obj client.Object, expectedKind string) {
result := GenerateOwnerReferenceFromObject(obj)

Expect(result.Kind).To(Equal(expectedKind))
Expect(result.APIVersion).To(Equal(datav1alpha1.GroupVersion.String()))
},

Entry("should recover the kind of a dataset",
&datav1alpha1.Dataset{
ObjectMeta: metav1.ObjectMeta{Name: "no-typemeta-dataset", Namespace: "default", UID: "uid-1"},
},
"Dataset",
),

Entry("should recover the kind of an alluxio runtime",
&datav1alpha1.AlluxioRuntime{
ObjectMeta: metav1.ObjectMeta{Name: "no-typemeta-runtime", Namespace: "default", UID: "uid-2"},
},
"AlluxioRuntime",
),

Entry("should recover the kind of a data load",
&datav1alpha1.DataLoad{
ObjectMeta: metav1.ObjectMeta{Name: "no-typemeta-dataload", Namespace: "default", UID: "uid-3"},
},
"DataLoad",
),

Entry("should recover the apiVersion when only the kind is set",
&datav1alpha1.DataLoad{
TypeMeta: metav1.TypeMeta{Kind: "DataLoad"},
ObjectMeta: metav1.ObjectMeta{Name: "kind-only-dataload", Namespace: "default", UID: "uid-4"},
},
"DataLoad",
),

Entry("should recover the kind when only the apiVersion is set",
&datav1alpha1.DataLoad{
TypeMeta: metav1.TypeMeta{APIVersion: datav1alpha1.GroupVersion.String()},
ObjectMeta: metav1.ObjectMeta{Name: "version-only-dataload", Namespace: "default", UID: "uid-5"},
},
"DataLoad",
),
)

It("should leave the reference incomplete for a type the fluid scheme does not know", func() {
// The helper has no error return, so a type missing from the scheme can only be reported through the
// log. Callers all pass registered fluid types today, this only guards against a future one that does
// not.
result := GenerateOwnerReferenceFromObject(&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: "unregistered", Namespace: "default", UID: "uid-6"},
})

Expect(result.Kind).To(BeEmpty())
Expect(result.Name).To(Equal("unregistered"))
})
})

var _ = Describe("FilterOwnerByKind", func() {
Expand Down
Loading