SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3 #976

ianshen1104 · 2022-07-07T03:17:58Z

What is this PR for?

Add app.kubernetes.io labels recommended here by Kubernetes
https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/

What type of PR is it?

Improvement

Todos

- Add app.kubernetes.io labels in submarine-cloud-v3 artifacts

What is the Jira issue?

https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-1290

How should this be tested?

Follow the submarine-cloud-v3/docs/developer-guide.md

Screenshots (if appropriate)

Questions:

Do the license files need updating? No
Are there breaking changes for older versions? No
Does this need new documentation? No

cdmikechen · 2022-07-08T01:21:56Z

Hi~ Thanks for your contribution.
Can we replace app.kubernetes.io/version value using a consistent method? I personally don't think hard-coding the version is a good way.
Another question is whether we have any way to test the current updates?

ianshen1104 · 2022-07-08T02:25:39Z

@cdmikechen thanks for your feedback!
I will update my changes soon and push again.

codecov · 2022-07-09T05:26:31Z

Codecov Report

Merging #976 (fcfd6a6) into master (e7ed4a5) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##             master    #976   +/-   ##
========================================
  Coverage      9.18%   9.18%           
  Complexity      670     670           
========================================
  Files           234     234           
  Lines         23716   23716           
  Branches       3475    3475           
========================================
  Hits           2178    2178           
  Misses        21407   21407           
  Partials        131     131

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

MortalHappiness · 2022-07-14T13:54:05Z

submarine-cloud-v3/artifacts/submarine-database.yaml

 kind: PersistentVolumeClaim
 metadata:
  name: submarine-database-pvc
+  labels:
+    app.kubernetes.io/name: submarine-database
+    app.kubernetes.io/version: "0.8.0-SNAPSHOT"
+    app.kubernetes.io/component: database
 spec:


I think we can write the version information in golang file instead of yaml file. For example, for this PersistentVolumeClaim, see the golang function below. We can set pvc.Labels before we create the resource. This version information can be written as golang constant.

submarine/submarine-cloud-v3/controllers/submarine_database.go

Lines 35 to 46 in e7ed4a5

func (r *SubmarineReconciler) newSubmarineDatabasePersistentVolumeClaim(ctx context.Context, submarine *submarineapacheorgv1alpha1.Submarine) *corev1.PersistentVolumeClaim {

pvc, err := ParsePersistentVolumeClaimYaml(databaseYamlPath)

if err != nil {

r.Log.Error(err, "ParsePersistentVolumeClaimYaml")

}

pvc.Namespace = submarine.Namespace

err = controllerutil.SetControllerReference(submarine, pvc, r.Scheme)

if err != nil {

r.Log.Error(err, "Set PVC ControllerReference")

}

return pvc

}

For your reference:

https://pkg.go.dev/k8s.io/api/core/v1#PersistentVolumeClaim

https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#ObjectMeta

Thank you for your suggestion @MortalHappiness,
I have changed the code to perform label setting in golang functions instead of yaml files.

Set objectMeta.Labels in all controller files

…o SUBMARINE-1290

…ulsets

xunliu

@ianshen1104 Thank you for your contributions. :-)

xunliu · 2022-09-06T05:01:52Z

submarine-cloud-v3/config/crd/bases/submarine.apache.org_submarines.yaml

-#
-# Licensed to the Apache Software Foundation (ASF) under one or more
-# contributor license agreements.  See the NOTICE file distributed with
-# this work for additional information regarding copyright ownership.
-# The ASF licenses this file to You under the Apache License, Version 2.0
-# (the "License"); you may not use this file except in compliance with
-# the License.  You may obtain a copy of the License at
-#
-#    http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-


hi @ianshen1104
The yaml file also needs to retain the Apache license description.
What is the reason for deletion?

xunliu · 2022-09-06T05:02:09Z

submarine-cloud-v3/config/rbac/role.yaml

-#
-# Licensed to the Apache Software Foundation (ASF) under one or more
-# contributor license agreements.  See the NOTICE file distributed with
-# this work for additional information regarding copyright ownership.
-# The ASF licenses this file to You under the Apache License, Version 2.0
-# (the "License"); you may not use this file except in compliance with
-# the License.  You may obtain a copy of the License at
-#
-#    http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#


The yaml file also needs to retain the Apache license description.

ianshen1104 added 2 commits July 7, 2022 11:05

SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3

0d5c54d

SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3

1970c71

MortalHappiness requested changes Jul 14, 2022

View reviewed changes

ianshen1104 added 5 commits July 20, 2022 13:28

Added appVersion const in submarine_controller

b269024

Set objectMeta.Labels in all controller files

Merge branch 'SUBMARINE-1290' of github.com:ianshen1104/submarine int…

78ee285

…o SUBMARINE-1290

Took out changes to yaml from old commit

a005a9c

Set pod labels through spec.template.labels of deployments and statef…

e0c7588

…ulsets

Changed label setting to mutative in remaining controllers

fcfd6a6

xunliu reviewed Sep 6, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3 #976

SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3 #976

ianshen1104 commented Jul 7, 2022

cdmikechen commented Jul 8, 2022 •

edited

ianshen1104 commented Jul 8, 2022

codecov bot commented Jul 9, 2022 •

edited

MortalHappiness Jul 14, 2022

ianshen1104 Aug 2, 2022

xunliu left a comment

xunliu Sep 6, 2022

xunliu Sep 6, 2022

	func (r SubmarineReconciler) newSubmarineDatabasePersistentVolumeClaim(ctx context.Context, submarine submarineapacheorgv1alpha1.Submarine) *corev1.PersistentVolumeClaim {
	pvc, err := ParsePersistentVolumeClaimYaml(databaseYamlPath)
	if err != nil {
	r.Log.Error(err, "ParsePersistentVolumeClaimYaml")
	}
	pvc.Namespace = submarine.Namespace
	err = controllerutil.SetControllerReference(submarine, pvc, r.Scheme)
	if err != nil {
	r.Log.Error(err, "Set PVC ControllerReference")
	}
	return pvc
	}

SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3 #976

Are you sure you want to change the base?

SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3 #976

Conversation

ianshen1104 commented Jul 7, 2022

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

cdmikechen commented Jul 8, 2022 • edited

ianshen1104 commented Jul 8, 2022

codecov bot commented Jul 9, 2022 • edited

Codecov Report

MortalHappiness Jul 14, 2022

Choose a reason for hiding this comment

ianshen1104 Aug 2, 2022

Choose a reason for hiding this comment

xunliu left a comment

Choose a reason for hiding this comment

xunliu Sep 6, 2022

Choose a reason for hiding this comment

xunliu Sep 6, 2022

Choose a reason for hiding this comment

cdmikechen commented Jul 8, 2022 •

edited

codecov bot commented Jul 9, 2022 •

edited