Skip to content

Commit

Permalink
hypershift-operator/controllers/hostedcluster: isUpgradeable minor-bu…
Browse files Browse the repository at this point in the history
…mp scoping

Godocs for Upgradeable [1]:

  Upgradeable indicates whether the component (operator and all
  configured operands) is safe to upgrade based on the current cluster
  state. When Upgradeable is False, the cluster-version operator will
  prevent the cluster from performing impacted updates unless forced.
  When set on ClusterVersion, the message will explain which updates
  (minor or patch) are impacted. When set on ClusterOperator, False
  will block minor OpenShift updates. The message field should contain
  a human readable description of what the administrator should do to
  allow the cluster or component to successfully update. The
  cluster-version operator will allow updates when this condition is
  not False, including when it is missing, True, or Unknown.

So we specifically doc it as only about 4.y -> 4.(y+1) minor updates
when seen on ClusterOperator.  But we leave it unclear on
ClusterVersion because when you set some ClusterVersion overrides, it
can break patch updates, so QE asked us to also block patch updates in
that case [2,3].

With this patch, I'm using availableUpdates and conditionalUpdates to
look up a version associated with the proposed target release
pullspec.  That's a bit less reliable than the current cluster-version
operator behavior, which is extracting the proposed target version
from the proposed release image itself (e.g. see [4]).  But it's
probably sufficient for now, with the odds that the OpenShift Update
Service serves bad data low.  And we can refine further in the future
if we want.

[1]: https://github.com/openshift/api/blob/cce310ad2932f6de24491052d506926e484c082c/config/v1/types_cluster_operator.go#L179-L190 :
[2]: openshift/cluster-version-operator#364
[3]: https://bugzilla.redhat.com/show_bug.cgi?id=1822844
[4]: openshift/cluster-version-operator#431
  • Loading branch information
wking committed Mar 28, 2023
1 parent 040d92b commit 5369cd8
Showing 1 changed file with 29 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4214,10 +4214,39 @@ func (r *HostedClusterReconciler) reconcileAWSSubnets(ctx context.Context, creat
return nil
}

func releaseImageToVersion(hcluster *hyperv1.HostedCluster, image string) (semver.Version, error) {
for _, update := range hcluster.Status.Version.AvailableUpdates {
if update.Image == image { // TODO: all we care about is matching digests
return semver.Parse(update.Version)
}
}
for _, update := range hcluster.Status.Version.ConditionalUpdates {
if update.Release.Image == image { // TODO: all we care about is matching digests
return semver.Parse(update.Release.Version)
}
}
return nil, fmt.Errorf("unable to determine a version string for %s", image)
}

func isUpgradeable(hcluster *hyperv1.HostedCluster) (bool, string, error) {
if hcluster.Status.Version != nil && hcluster.Status.Version.Desired.Image != hcluster.Spec.Release.Image {
upgradeable := meta.FindStatusCondition(hcluster.Status.Conditions, string(hyperv1.ClusterVersionUpgradeable))
if upgradeable != nil && upgradeable.Status == metav1.ConditionFalse {
currentTargetVersion, err := semver.Parse(hcluster.Status.Version.Desired.Version)
if err != nil {
return fmt.Errorf("cluster is %s=%s (%s: %s), and failed to parse the current target %s as a Semantic Version: %w", upgradeable.Type, upgradeable.Status, upgradeable.Reason, upgradeable.Message, hcluster.Status.Version.Desired.Version, error)
}

requestedVersion, err := releaseImageToVersion(hcluster, hcluster.Spec.Release.Image)
if err != nil {
return fmt.Errorf("cluster is %s=%s (%s: %s), and failed determine a version for %s to see if it is a minor update from %s: %w", upgradeable.Type, upgradeable.Status, upgradeable.Reason, upgradeable.Message, hcluster.Spec.Release.Image, hcluster.Status.Version.Desired.Version, err)
}

if requestedVersion.Major == currentTargetVersion.Major && requestedVersion.Minor == currentTargetVersion.Minor {
// ClusterVersion's Upgradeable condition is mostly about minor bumps from x.y to x.(y+1) and larger. It is not intended to block patch updates from x.y.z to x.y.z' except under very limited circumstances which we can ignore for now.
return true, "", nil
}

releaseImage, exists := hcluster.Annotations[hyperv1.ForceUpgradeToAnnotation]
if !exists {
return true, "", fmt.Errorf("cluster version is not upgradeable")
Expand Down

0 comments on commit 5369cd8

Please sign in to comment.