Skip to content

Add (and use) full version label to ClusterDeployment #2084

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

Merged
openshift-merge-robot merged 1 commit into from
Aug 11, 2023
Merged

Add (and use) full version label to ClusterDeployment #2084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 apis/hive/v1/clusterdeployment_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ const InitializedConditionReason = "Initialized"
// +kubebuilder:printcolumn:name="InfraID",type="string",JSONPath=".spec.clusterMetadata.infraID"
// +kubebuilder:printcolumn:name="Platform",type="string",JSONPath=".metadata.labels.hive\\.openshift\\.io/cluster-platform"
// +kubebuilder:printcolumn:name="Region",type="string",JSONPath=".metadata.labels.hive\\.openshift\\.io/cluster-region"
// +kubebuilder:printcolumn:name="Version",type="string",JSONPath=".metadata.labels.hive\\.openshift\\.io/version-major-minor-patch"
// +kubebuilder:printcolumn:name="Version",type="string",JSONPath=".metadata.labels.hive\\.openshift\\.io/version"
// +kubebuilder:printcolumn:name="ClusterType",type="string",JSONPath=".metadata.labels.hive\\.openshift\\.io/cluster-type"
// +kubebuilder:printcolumn:name="ProvisionStatus",type="string",JSONPath=".status.conditions[?(@.type=='Provisioned')].reason"
// +kubebuilder:printcolumn:name="PowerState",type="string",JSONPath=".status.powerState"
Expand Down
2 changes: 1 addition & 1 deletion config/crds/hive.openshift.io_clusterdeployments.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ spec:
- jsonPath: .metadata.labels.hive\.openshift\.io/cluster-region
name: Region
type: string
- jsonPath: .metadata.labels.hive\.openshift\.io/version-major-minor-patch
Copy link
Copy Markdown
Contributor

@suhanime suhanime Aug 9, 2023

Choose a reason for hiding this comment

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

Are we completely sure no one uses this label before we remove it? From what I understand, the practice is to keep both and let the user decide which one they want to use

Copy link
Copy Markdown
Member Author

@2uasimojo 2uasimojo Aug 9, 2023

Choose a reason for hiding this comment

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

We’re not removing the label, just changing which label we use in oc get cd

Copy link
Copy Markdown
Member Author

@2uasimojo 2uasimojo Aug 9, 2023

Choose a reason for hiding this comment

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

To expand a bit: 99% of clusters (and 100% of prod clusters, since they should only be using GAed releases) will have the same value here before & after.

- jsonPath: .metadata.labels.hive\.openshift\.io/version
name: Version
type: string
- jsonPath: .metadata.labels.hive\.openshift\.io/cluster-type
Expand Down
2 changes: 1 addition & 1 deletion hack/app-sre/saas-template.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ objects:
- jsonPath: .metadata.labels.hive\.openshift\.io/cluster-region
name: Region
type: string
- jsonPath: .metadata.labels.hive\.openshift\.io/version-major-minor-patch
- jsonPath: .metadata.labels.hive\.openshift\.io/version
name: Version
type: string
- jsonPath: .metadata.labels.hive\.openshift\.io/cluster-type
Expand Down
5 changes: 5 additions & 0 deletions pkg/constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,11 @@ const (
// VSphereCertificatesDir is the directory containing VSphere certificate files.
VSphereCertificatesDir = "/vsphere-certificates"

// VersionLabel is a label applied to ClusterDeployments to show the full version of the cluster
// per ClusterVersion.status.desired.version. This should differ from VersionMajorMinorPatchLabel
// only for pre-release versions.
VersionLabel = "hive.openshift.io/version"

// VersionMajorLabel is a label applied to ClusterDeployments to show the version of the cluster
// in the form "[MAJOR]".
VersionMajorLabel = "hive.openshift.io/version-major"
Expand Down
20 changes: 13 additions & 7 deletions pkg/controller/clusterversion/clusterversion_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,26 +161,32 @@ func (r *ReconcileClusterVersion) Reconcile(ctx context.Context, request reconci

func (r *ReconcileClusterVersion) updateClusterVersionLabels(cd *hivev1.ClusterDeployment, clusterVersion *openshiftapiv1.ClusterVersion, cdLog log.FieldLogger) error {
changed := false
if version, err := semver.ParseTolerant(clusterVersion.Status.Desired.Version); err == nil {
if cd.Labels == nil {
cd.Labels = make(map[string]string, 3)
}
cvoVersion := clusterVersion.Status.Desired.Version
if cd.Labels == nil {
cd.Labels = map[string]string{}
}
if cd.Labels[constants.VersionLabel] != cvoVersion {
cd.Labels[constants.VersionLabel] = cvoVersion
changed = true
}
if version, err := semver.ParseTolerant(cvoVersion); err == nil {
major := fmt.Sprintf("%d", version.Major)
majorMinor := fmt.Sprintf("%s.%d", major, version.Minor)
majorMinorPatch := fmt.Sprintf("%s.%d", majorMinor, version.Patch)
changed = cd.Labels[constants.VersionMajorLabel] != major ||
changed = changed ||
cd.Labels[constants.VersionMajorLabel] != major ||
cd.Labels[constants.VersionMajorMinorLabel] != majorMinor ||
cd.Labels[constants.VersionMajorMinorPatchLabel] != majorMinorPatch
cd.Labels[constants.VersionMajorLabel] = major
cd.Labels[constants.VersionMajorMinorLabel] = majorMinor
cd.Labels[constants.VersionMajorMinorPatchLabel] = majorMinorPatch
} else {
cdLog.WithField("version", clusterVersion.Status.Desired.Version).WithError(err).Warn("could not parse the cluster version")
cdLog.WithField("version", cvoVersion).WithError(err).Warn("could not parse the cluster version")
origLen := len(cd.Labels)
delete(cd.Labels, constants.VersionMajorLabel)
delete(cd.Labels, constants.VersionMajorMinorLabel)
delete(cd.Labels, constants.VersionMajorMinorPatchLabel)
changed = origLen != len(cd.Labels)
changed = changed || origLen != len(cd.Labels)
}

if !changed {
Expand Down
1 change: 1 addition & 0 deletions pkg/controller/clusterversion/clusterversion_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func TestClusterVersionReconcile(t *testing.T) {
testKubeconfigSecret(),
},
validate: func(t *testing.T, cd *hivev1.ClusterDeployment) {
assert.Equal(t, "2.3.4+somebuild", cd.Labels[constants.VersionLabel], "unexpected version label")
assert.Equal(t, "2", cd.Labels[constants.VersionMajorLabel], "unexpected version major label")
assert.Equal(t, "2.3", cd.Labels[constants.VersionMajorMinorLabel], "unexpected version major-minor label")
assert.Equal(t, "2.3.4", cd.Labels[constants.VersionMajorMinorPatchLabel], "unexpected version major-minor-patch label")
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/hibernation/hibernation_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ func deleteTransitionMetric(metric *prometheus.HistogramVec, cd *hivev1.ClusterD
"cluster_deployment_namespace": cd.Namespace,
"cluster_deployment": cd.Name,
"platform": cd.Labels[hivev1.HiveClusterPlatformLabel],
"cluster_version": cd.Labels[constants.VersionMajorMinorPatchLabel],
"cluster_version": cd.Labels[constants.VersionLabel],
"cluster_pool_namespace": poolNS,
})
}
Expand All @@ -675,7 +675,7 @@ func logCumulativeMetric(metric *prometheus.HistogramVec, cd *hivev1.ClusterDepl
poolNS = cd.Spec.ClusterPoolRef.Namespace
poolName = cd.Spec.ClusterPoolRef.PoolName
}
metric.WithLabelValues(cd.Labels[constants.VersionMajorMinorPatchLabel],
metric.WithLabelValues(cd.Labels[constants.VersionLabel],
cd.Labels[hivev1.HiveClusterPlatformLabel],
poolNS,
poolName).Observe(time)
Expand Down
6 changes: 3 additions & 3 deletions pkg/controller/machinepool/azureactuator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestAzureActuator(t *testing.T) {
clusterDeployment: testAzureClusterDeployment(),
pool: func() *hivev1.MachinePool {
p := testAzurePool()
p.Spec.Replicas = pointer.Int64Ptr(5)
p.Spec.Replicas = pointer.Int64(5)
return p
}(),
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
Expand Down Expand Up @@ -311,7 +311,7 @@ func TestAzureActuator(t *testing.T) {
clusterDeployment: testAzureClusterDeployment412(),
pool: func() *hivev1.MachinePool {
p := testAzurePool()
p.Spec.Replicas = pointer.Int64Ptr(5)
p.Spec.Replicas = pointer.Int64(5)
return p
}(),
mockAzureClient: func(mockCtrl *gomock.Controller, client *mockazure.MockClient) {
Expand Down Expand Up @@ -595,7 +595,7 @@ func testAzureClusterDeployment() *hivev1.ClusterDeployment {

func testAzureClusterDeployment412() *hivev1.ClusterDeployment {
cd := testAzureClusterDeployment()
cd.Labels[constants.VersionMajorMinorPatchLabel] = "4.12.0"
cd.Labels[constants.VersionLabel] = "4.12.0"
return cd
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/controller/machinepool/machinepool_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,7 @@ func getMinMaxReplicasForMachineSet(pool *hivev1.MachinePool, machineSets []*mac
}

func getClusterVersion(cd *hivev1.ClusterDeployment) (string, error) {
version, versionPresent := cd.Labels[constants.VersionMajorMinorPatchLabel]
version, versionPresent := cd.Labels[constants.VersionLabel]
if !versionPresent {
return "", errors.New("cluster version not set in clusterdeployment")
}
Expand All @@ -1154,13 +1154,13 @@ func platformAllowsZeroAutoscalingMinReplicas(cd *hivev1.ClusterDeployment) bool

// Since 4.7, OpenStack allows zero-sized minReplicas for autoscaling
if cd.Spec.Platform.OpenStack != nil {
majorMinorPatch, ok := cd.Labels[constants.VersionMajorMinorPatchLabel]
version, ok := cd.Labels[constants.VersionLabel]
if !ok {
// can't determine whether to allow zero minReplicas
return false
}

currentVersion, err := semver.Make(majorMinorPatch)
currentVersion, err := semver.Make(version)
if err != nil {
// assume we can't set minReplicas to zero
return false
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/machinepool/machinepool_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@ func testClusterDeployment() *hivev1.ClusterDeployment {
Finalizers: []string{hivev1.FinalizerDeprovision},
UID: types.UID("1234"),
Labels: map[string]string{
constants.VersionMajorMinorPatchLabel: "4.4.0",
constants.VersionLabel: "4.4.0",
},
},
Spec: hivev1.ClusterDeploymentSpec{
Expand Down Expand Up @@ -1480,6 +1480,6 @@ func withClusterVersion(cd *hivev1.ClusterDeployment, version string) *hivev1.Cl
if cd.Labels == nil {
cd.Labels = make(map[string]string, 1)
}
cd.Labels[constants.VersionMajorMinorPatchLabel] = version
cd.Labels[constants.VersionLabel] = version
return cd
}
2 changes: 1 addition & 1 deletion pkg/controller/metrics/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ func logHistogramDurationMetric(metric *prometheus.HistogramVec, cd *hivev1.Clus
cd.Namespace,
cd.Name,
cd.Labels[hivev1.HiveClusterPlatformLabel],
cd.Labels[constants.VersionMajorMinorPatchLabel],
cd.Labels[constants.VersionLabel],
poolNS).Observe(time)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/test/clusterdeployment/clusterdeployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func InstallRestarts(restarts int) Option {
}

func WithClusterVersion(version string) Option {
return Generic(generic.WithLabel(constants.VersionMajorMinorPatchLabel, version))
return Generic(generic.WithLabel(constants.VersionLabel, version))
}

func WithPowerState(powerState hivev1.ClusterPowerState) Option {
Expand Down
2 changes: 1 addition & 1 deletion vendor/github.com/openshift/hive/apis/hive/v1/clusterdeployment_types.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.