Skip to content
Permalink
Browse files

Reworked chart version range resolution strategy refs #62

This change is aiming to address the problem we encountered during
exploiting release-0.5. In this release we introduced helm chart range
functionality, where an application.spec.template.chart.version could
contain a SemVer constraint and be resolved to a material chart version.
In the original implementation we decided to keep
application.spec.template.chart.version as given and preserve resolved
chart version in release.spec.environment.chart.version. Under these
circumstances we broke 2 things: normal rollout and rollbacks.

Rollouts were broken due to an always-mismatch between env hashes of the
application and it's contender: due to the difference between the
versions, env hashes were never the same and therefore Shipper was
continuously re-creating new releases. On top of that, an attempt to
re-rollout the same app with a constraint would fail as well even if we
had fixed the hash problem: they would be the same, which Shipper
recofnises as a no-op.

Rollbacks were breaking the contract due to the environment back-copy
operations: during a rollback Shipper copies contender spec back to the
application. This meant we're loosing the initial information about the
original SemVer constraint.

This change takes a different approach. In essence,
application.spec.template.chart.version is being updated on the release
stage: the chart version is being materialised and preserved in the
application spec instead of the initial constraint. The constraint is
now preserved in application annotations among with the chart name and
the resolved version. These 3 components are being used to detect
whether a specific chart version has already been resolved and whether
it's consistent with the one specified in the spec. In this case the
rest of the algorithm stays in place: releases are being created with
the updated materialised chart spec, and rollbacks don't loose any data.

Signed-off-by: Oleg Sidorov <oleg.sidorov@booking.com>
  • Loading branch information...
icanhazbroccoli committed Jul 31, 2019
1 parent 5b0b175 commit f1ab8c613488e6c5b15335dc31cd37de15564597
45 go.sum

Large diffs are not rendered by default.

@@ -32,6 +32,10 @@ const (

AppHighestObservedGenerationAnnotation = "shipper.booking.com/app.highestObservedGeneration"

AppChartNameAnnotation = "shipper.booking.com/app.chart.name"
AppChartVersionResolvedAnnotation = "shipper.booking.com/app.chart.version.resolved"
AppChartVersionRawAnnotation = "shipper.booking.com/app.chart.version.raw"

ReleaseGenerationAnnotation = "shipper.booking.com/release.generation"
ReleaseTemplateIterationAnnotation = "shipper.booking.com/release.template.iteration"
ReleaseClustersAnnotation = "shipper.booking.com/release.clusters"
@@ -364,31 +364,42 @@ func (c *Controller) processApplication(app *shipper.Application) error {
c.cleanUpReleasesForApplication(app, appReleases)
}()

// Check if application chart spec is resolved: the original version
// might contain either a specific version or a semver constraint.
// If a semver constraint is found, it would be resolved in-place.
if !apputil.ChartVersionResolved(app) {
if _, err := apputil.ResolveChartVersion(app, c.versionResolver); err != nil {
return err
}
}

if contender, err = apputil.GetContender(app.Name, appReleases); err != nil {
if shippererrors.IsContenderNotFoundError(err) {
// Contender doesn't exist, so we are covering the case where Shipper
// is creating the first release for this application.
var generation = 0
if releaseName, iteration, err := c.releaseNameForApplication(app); err != nil {
return err
} else if rel, err := c.createReleaseForApplication(app, releaseName, iteration, generation); err != nil {
releaseSyncedCond := apputil.NewApplicationCondition(
shipper.ApplicationConditionTypeReleaseSynced,
corev1.ConditionFalse,
conditions.CreateReleaseFailed,
fmt.Sprintf("could not create a new release: %q", err))
apputil.SetApplicationCondition(&app.Status, *releaseSyncedCond)
return err
} else {
appReleases = append(appReleases, rel)
}
// It seems that adding an object to the fakeClient doesn't
// update listers and informers automatically during tests...
// How should we do it then?
apputil.SetHighestObservedGeneration(app, generation)
return c.wrapUpApplicationConditions(app, appReleases)
// Anything else rather than not found err is an abort case
if !shippererrors.IsContenderNotFoundError(err) {
return err
}
return err

// Contender doesn't exist, so we are covering the case where Shipper
// is creating the first release for this application.
var generation = 0
if releaseName, iteration, err := c.releaseNameForApplication(app); err != nil {
return err
} else if rel, err := c.createReleaseForApplication(app, releaseName, iteration, generation); err != nil {
releaseSyncedCond := apputil.NewApplicationCondition(
shipper.ApplicationConditionTypeReleaseSynced,
corev1.ConditionFalse,
conditions.CreateReleaseFailed,
fmt.Sprintf("could not create a new release: %q", err))
apputil.SetApplicationCondition(&app.Status, *releaseSyncedCond)
return err
} else {
appReleases = append(appReleases, rel)
}
// It seems that adding an object to the fakeClient doesn't
// update listers and informers automatically during tests...
// How should we do it then?
apputil.SetHighestObservedGeneration(app, generation)
return c.wrapUpApplicationConditions(app, appReleases)
}

if generation, err = releaseutil.GetGeneration(contender); err != nil {
@@ -409,6 +420,8 @@ func (c *Controller) processApplication(app *shipper.Application) error {
// created and deleted. As side-effect of this, the contender's
// environment will be copied back to the application.
apputil.CopyEnvironment(app, contender)
// keeping app annotations consistent with the new "old" release
apputil.UpdateChartVersionResolvedAnnotation(app, contender.Spec.Environment.Chart.Version)
apputil.SetHighestObservedGeneration(app, generation)
abortingCond := apputil.NewApplicationCondition(shipper.ApplicationConditionTypeAborting, corev1.ConditionTrue, "", fmt.Sprintf("abort in progress, returning state to release %q", contender.Name))
apputil.SetApplicationCondition(&app.Status, *abortingCond)

0 comments on commit f1ab8c6

Please sign in to comment.
You can’t perform that action at this time.