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.

Sign up for GitHub

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

rename FleetCluster to KubeConfigSecretRef #484

Merged
merged 2 commits into from
Nov 29, 2023
Merged

rename FleetCluster to KubeConfigSecretRef #484

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
83fdec0
rename FleetCluster to RenderableFleetCluster
Xieql Nov 25, 2023
5de1517
refactor name to KubeConfigsecretRef
Xieql Nov 28, 2023
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
2 changes: 1 addition & 1 deletion pkg/fleet-manager/fleet_plugin_backup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (f *FleetManager) reconcileBackupPlugin(ctx context.Context, fleet *v1alpha
// Iterating through each fleet cluster to generate and apply Velero helm configurations.
for key, cluster := range fleetClusters {
// generate Velero helm config for each fleet cluster
b, err := plugin.RenderVelero(f.Manifests, fleetNN, fleetOwnerRef, plugin.FleetCluster{
b, err := plugin.RenderVelero(f.Manifests, fleetNN, fleetOwnerRef, plugin.RenderableFleetCluster{
Copy link
Member

Choose a reason for hiding this comment

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

can we use the FleetCluster, if any field miss, we can add it

Copy link
Contributor Author

@Xieql Xieql Nov 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

the origin FleetCluster 's fileld value is comes from two struct fleetCluster and ClusterKey ,
so maybe we can not directly add fileld in FleetCluster, or we need merge those two struct into one struct, which may affect many other code

kurator/pkg/fleet-manager/fleet_cluster.go

Lines 34 to 43 in 2871c99

type fleetCluster struct {
Secret string
SecretKey string
client *kclient.Client
}
type ClusterKey struct {
Kind string
Name string
}

this is the most used usage:

kurator/pkg/fleet-manager/fleet_plugin_distributedstorage.go

Lines 78 to 81 in 83fdec0

b, err := plugin.RenderClusterStorage(f.Manifests, fleetNN, fleetOwnerRef, plugin.RenderableFleetCluster{
Name: key.Name,
SecretName: cluster.Secret,
SecretKey: cluster.SecretKey, 

FleetCluster.Name:       ClusterKey.Name, 
FleetCluster.SecretName: fleetCluster.Secret, 
FleetCluster.SecretKey:  fleetCluster.SecretKey,

Name: key.Name,
SecretName: cluster.Secret,
SecretKey: cluster.SecretKey,
Expand Down
4 changes: 2 additions & 2 deletions pkg/fleet-manager/fleet_plugin_distributedstorage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (f *FleetManager) reconcileDistributedStoragePlugin(ctx context.Context, fl

// First install rook-operator for the specified multicluster.
for key, cluster := range fleetClusters {
b, err := plugin.RendeStorageOperator(f.Manifests, fleetNN, fleetOwnerRef, plugin.FleetCluster{
b, err := plugin.RendeStorageOperator(f.Manifests, fleetNN, fleetOwnerRef, plugin.RenderableFleetCluster{
Name: key.Name,
SecretName: cluster.Secret,
SecretKey: cluster.SecretKey,
Expand Down Expand Up @@ -75,7 +75,7 @@ func (f *FleetManager) reconcileDistributedStoragePlugin(ctx context.Context, fl
// After Rook operator are created, starts to install rook-ceph
if distributedStorageCfg.Storage != nil {
for key, cluster := range fleetClusters {
b, err := plugin.RenderClusterStorage(f.Manifests, fleetNN, fleetOwnerRef, plugin.FleetCluster{
b, err := plugin.RenderClusterStorage(f.Manifests, fleetNN, fleetOwnerRef, plugin.RenderableFleetCluster{
Name: key.Name,
SecretName: cluster.Secret,
SecretKey: cluster.SecretKey,
Expand Down
4 changes: 2 additions & 2 deletions pkg/fleet-manager/fleet_plugin_kyverno.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (f *FleetManager) reconcileKyvernoPlugin(ctx context.Context, fleet *fleetv
fleetOwnerRef := ownerReference(fleet)
var resources kube.ResourceList
for key, cluster := range fleetClusters {
b, err := plugin.RenderKyverno(f.Manifests, fleetNN, fleetOwnerRef, plugin.FleetCluster{
b, err := plugin.RenderKyverno(f.Manifests, fleetNN, fleetOwnerRef, plugin.RenderableFleetCluster{
Name: key.Name,
SecretName: cluster.Secret,
SecretKey: cluster.SecretKey,
Expand Down Expand Up @@ -78,7 +78,7 @@ func (f *FleetManager) reconcileKyvernoPlugin(ctx context.Context, fleet *fleetv
if kyvernoCfg.PodSecurity != nil {
for key, cluster := range fleetClusters {
// generate policies for pod security admission
b, err := plugin.RenderKyvernoPolicy(f.Manifests, fleetNN, fleetOwnerRef, plugin.FleetCluster{
b, err := plugin.RenderKyvernoPolicy(f.Manifests, fleetNN, fleetOwnerRef, plugin.RenderableFleetCluster{
Name: key.Name,
SecretName: cluster.Secret,
SecretKey: cluster.SecretKey,
Expand Down
2 changes: 1 addition & 1 deletion pkg/fleet-manager/fleet_plugin_metric.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func (f *FleetManager) reconcileMetricPlugin(ctx context.Context, fleet *fleetap
return nil, ctrl.Result{}, fmt.Errorf("failed to reconcile objstore secret for cluster %s: %w", c.Name, err)
}

b, err := plugin.RenderPrometheus(f.Manifests, fleetNN, fleetOwnerRef, plugin.FleetCluster{
b, err := plugin.RenderPrometheus(f.Manifests, fleetNN, fleetOwnerRef, plugin.RenderableFleetCluster{
Name: c.Name,
SecretName: fleetCluster.Secret,
SecretKey: fleetCluster.SecretKey,
Expand Down
12 changes: 6 additions & 6 deletions pkg/fleet-manager/plugin/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type GrafanaDataSource struct {
IsDefault bool `json:"isDefault"`
}

func RenderKyvernoPolicy(fsys fs.FS, fleetNN types.NamespacedName, fleetRef *metav1.OwnerReference, cluster FleetCluster, kyvernoCfg *fleetv1a1.KyvernoConfig) ([]byte, error) {
func RenderKyvernoPolicy(fsys fs.FS, fleetNN types.NamespacedName, fleetRef *metav1.OwnerReference, cluster RenderableFleetCluster, kyvernoCfg *fleetv1a1.KyvernoConfig) ([]byte, error) {
c, err := getFleetPluginChart(fsys, KyvernoPolicyComponentName)
if err != nil {
return nil, err
Expand All @@ -84,7 +84,7 @@ func RenderKyvernoPolicy(fsys fs.FS, fleetNN types.NamespacedName, fleetRef *met
})
}

func RenderKyverno(fsys fs.FS, fleetNN types.NamespacedName, fleetRef *metav1.OwnerReference, cluster FleetCluster, kyvernoCfg *fleetv1a1.KyvernoConfig) ([]byte, error) {
func RenderKyverno(fsys fs.FS, fleetNN types.NamespacedName, fleetRef *metav1.OwnerReference, cluster RenderableFleetCluster, kyvernoCfg *fleetv1a1.KyvernoConfig) ([]byte, error) {
c, err := getFleetPluginChart(fsys, KyvernoComponentName)
if err != nil {
return nil, err
Expand Down Expand Up @@ -181,7 +181,7 @@ func RenderThanos(fsys fs.FS, fleetNN types.NamespacedName, fleetRef *metav1.Own
return renderFleetPlugin(fsys, thanosCfg)
}

func RenderPrometheus(fsys fs.FS, fleetName types.NamespacedName, fleetRef *metav1.OwnerReference, cluster FleetCluster, metricCfg *fleetv1a1.MetricConfig) ([]byte, error) {
func RenderPrometheus(fsys fs.FS, fleetName types.NamespacedName, fleetRef *metav1.OwnerReference, cluster RenderableFleetCluster, metricCfg *fleetv1a1.MetricConfig) ([]byte, error) {
promChart, err := getFleetPluginChart(fsys, PrometheusComponentName)
if err != nil {
return nil, err
Expand Down Expand Up @@ -230,7 +230,7 @@ func RenderVelero(
fsys fs.FS,
fleetNN types.NamespacedName,
fleetRef *metav1.OwnerReference,
cluster FleetCluster,
cluster RenderableFleetCluster,
backupCfg *fleetv1a1.BackupConfig,
veleroSecretName string,
) ([]byte, error) {
Expand Down Expand Up @@ -305,7 +305,7 @@ func RendeStorageOperator(
fsys fs.FS,
fleetNN types.NamespacedName,
fleetRef *metav1.OwnerReference,
cluster FleetCluster,
cluster RenderableFleetCluster,
distributedStorageCfg *fleetv1a1.DistributedStorageConfig,
) ([]byte, error) {
// get and merge the chart config
Expand Down Expand Up @@ -336,7 +336,7 @@ func RenderClusterStorage(
fsys fs.FS,
fleetNN types.NamespacedName,
fleetRef *metav1.OwnerReference,
cluster FleetCluster,
cluster RenderableFleetCluster,
distributedStorageCfg *fleetv1a1.DistributedStorageConfig,
) ([]byte, error) {
c, err := getFleetPluginChart(fsys, RookClusterComponentName)
Expand Down
12 changes: 6 additions & 6 deletions pkg/fleet-manager/plugin/plugin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestRenderKyvernoPolicy(t *testing.T) {

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got, err := RenderKyvernoPolicy(manifestFS, tc.fleet, tc.ref, FleetCluster{
got, err := RenderKyvernoPolicy(manifestFS, tc.fleet, tc.ref, RenderableFleetCluster{
Name: "cluster1",
SecretName: "cluster1",
SecretKey: "kubeconfig.yaml",
Expand Down Expand Up @@ -98,7 +98,7 @@ func TestRenderKyverno(t *testing.T) {

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got, err := RenderKyverno(manifestFS, tc.fleet, tc.ref, FleetCluster{
got, err := RenderKyverno(manifestFS, tc.fleet, tc.ref, RenderableFleetCluster{
Name: "cluster1",
SecretName: "cluster1",
SecretKey: "kubeconfig.yaml",
Expand Down Expand Up @@ -270,7 +270,7 @@ func TestRenderPrometheus(t *testing.T) {

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got, err := RenderPrometheus(manifestFS, tc.fleet, tc.ref, FleetCluster{
got, err := RenderPrometheus(manifestFS, tc.fleet, tc.ref, RenderableFleetCluster{
Name: "cluster1",
SecretName: "cluster1",
SecretKey: "kubeconfig.yaml",
Expand Down Expand Up @@ -382,7 +382,7 @@ func TestRenderVelero(t *testing.T) {

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got, err := RenderVelero(manifestFS, tc.fleet, tc.ref, FleetCluster{
got, err := RenderVelero(manifestFS, tc.fleet, tc.ref, RenderableFleetCluster{
Name: "cluster1",
SecretName: "cluster1",
SecretKey: "kubeconfig.yaml",
Expand Down Expand Up @@ -437,7 +437,7 @@ func TestRenderStorageOperator(t *testing.T) {

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got, err := RendeStorageOperator(manifestFS, tc.fleet, tc.ref, FleetCluster{
got, err := RendeStorageOperator(manifestFS, tc.fleet, tc.ref, RenderableFleetCluster{
Name: "cluster1",
SecretName: "cluster1",
SecretKey: "kubeconfig.yaml",
Expand Down Expand Up @@ -527,7 +527,7 @@ func TestRenderClusterStorage(t *testing.T) {

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got, err := RenderClusterStorage(manifestFS, tc.fleet, tc.ref, FleetCluster{
got, err := RenderClusterStorage(manifestFS, tc.fleet, tc.ref, RenderableFleetCluster{
Name: "cluster1",
SecretName: "cluster1",
SecretKey: "kubeconfig.yaml",
Expand Down
4 changes: 2 additions & 2 deletions pkg/fleet-manager/plugin/render.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type FleetPluginConfig struct {
Component string
Fleet types.NamespacedName
OwnerReference *metav1.OwnerReference
Cluster *FleetCluster
Cluster *RenderableFleetCluster
Chart ChartConfig
Values map[string]interface{}
}
Expand All @@ -55,7 +55,7 @@ func (plugin FleetPluginConfig) StorageNamespace() string {
return plugin.Fleet.Namespace
}

type FleetCluster struct {
type RenderableFleetCluster struct {
Name string
Copy link
Member

Choose a reason for hiding this comment

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

nit: The Name can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, it is used solely for rendering and requires the name parameter.

If we remove name, we will need to refactor many corresponding functions

However, I think that refactoring them would not be wise.

such as

kurator/pkg/fleet-manager/plugin/render.go

Lines 30 to 46 in 5de1517

type FleetPluginConfig struct {
Name string
Component string
Fleet types.NamespacedName
OwnerReference *metav1.OwnerReference
Cluster *KubeConfigSecretRef
Chart ChartConfig
Values map[string]interface{}
}
func (plugin FleetPluginConfig) ResourceName() string {
if plugin.Cluster != nil {
return plugin.Component + "-" + plugin.Cluster.Name
}
return plugin.Component
}

Copy link
Member

Choose a reason for hiding this comment

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

I did see that, it is unused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right, it is unused

SecretName string
SecretKey string
Expand Down