-
Notifications
You must be signed in to change notification settings - Fork 252
SUBMARINE-1290. Add k8s labels to k8s resources in submarine-cloud-v3 #976
base: master
Are you sure you want to change the base?
Conversation
Hi~ Thanks for your contribution. |
@cdmikechen thanks for your feedback! |
Codecov Report
@@ 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 |
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: |
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.
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:
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.
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
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.
@ianshen1104 Thank you for your contributions. :-)
# | ||
# 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. | ||
# | ||
|
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.
hi @ianshen1104
The yaml file also needs to retain the Apache license
description.
What is the reason for deletion?
# | ||
# 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. | ||
# |
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.
The yaml file also needs to retain the Apache license
description.
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
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: