Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

Installation controller should recreate resources on application clusters if they don't exist #110

Closed
parhamdoustdar opened this issue Jul 2, 2019 · 0 comments · Fixed by #205
Closed

Installation controller should recreate resources on application clusters if they don't exist #110

parhamdoustdar opened this issue Jul 2, 2019 · 0 comments · Fixed by #205
Assignees
@juliogreff
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone
release-0.7

Comments

@parhamdoustdar
Copy link
Contributor

Right now, we only recreate resources for the last release in the release history. We should do this for all releases.

Here are the steps to reproduce a scenario we've seen in production:

  1. Set up an environment in which you have an Application object with at least 1 completed Release
  2. Start a new release by changing something in your Application object
  3. Go into the application cluster and delete the deployment belonging to the incumbent
  4. Try to advance the contender. You should see the condition incumbentAchievedCapacity turn to false, because when Shipper is looking for the deployment for it, there isn't any.

The way this should work is that when a resource belonging to a release is deleted, it should be put back the way it was.
juliogreff added a commit that referenced this issue Jul 9, 2019
@juliogreff
Make InstallationTarget objects self contained
cfac9c4 
Closes #5. You can read all of the context there too.

We want InstallationTarget objects to be self-contained, independent
from releases and applications. We're making them so by applying a few
changes:

1. The installation controller loses its smarts in regards to finding
out if the InstallationTarget is part of an incumbent or contender
release. This logic now belongs entiretly to the release controller.

2. The InstallationTarget becomes a richer object, by containing the
chart and values specified by the Application object at the time of the
creation of the release.

This commit takes a slightly different approach than the one described
in #5, though. Since we currently don't support installing different
things (either different charts or different values) per cluster with
the same InstallationTarget, we chose not to implement it here. That's
mostly in case YAGNI, and the cost of doing so in the future is roughly
the same as doing it now, so we took the call to punt it to future us.

We also decided to continue completely skipping InstallationTargets that
do not belong to a contender release (or that have CanOverride = false,
now), because that would slow down the installation controller too much
before #77 is closed. We captured that requirement in #110 though.
juliogreff added a commit that referenced this issue Jul 10, 2019
@juliogreff
Make InstallationTarget objects self contained
030acc4 
Closes #5. You can read all of the context there too.

We want InstallationTarget objects to be self-contained, independent
from releases and applications. We're making them so by applying a few
changes:

1. The installation controller loses its smarts in regards to finding
out if the InstallationTarget is part of an incumbent or contender
release. This logic now belongs entiretly to the release controller.

2. The InstallationTarget becomes a richer object, by containing the
chart and values specified by the Application object at the time of the
creation of the release.

This commit takes a slightly different approach than the one described
in #5, though. Since we currently don't support installing different
things (either different charts or different values) per cluster with
the same InstallationTarget, we chose not to implement it here. That's
mostly in case YAGNI, and the cost of doing so in the future is roughly
the same as doing it now, so we took the call to punt it to future us.

We also decided to continue completely skipping InstallationTargets that
do not belong to a contender release (or that have CanOverride = false,
now), because that would slow down the installation controller too much
before #77 is closed. We captured that requirement in #110 though.
juliogreff added a commit that referenced this issue Jul 11, 2019
@juliogreff
Make InstallationTarget objects self contained
7aa7c28 
Closes #5. You can read all of the context there too.

We want InstallationTarget objects to be self-contained, independent
from releases and applications. We're making them so by applying a few
changes:

1. The installation controller loses its smarts in regards to finding
out if the InstallationTarget is part of an incumbent or contender
release. This logic now belongs entiretly to the release controller.

2. The InstallationTarget becomes a richer object, by containing the
chart and values specified by the Application object at the time of the
creation of the release.

This commit takes a slightly different approach than the one described
in #5, though. Since we currently don't support installing different
things (either different charts or different values) per cluster with
the same InstallationTarget, we chose not to implement it here. That's
mostly in case YAGNI, and the cost of doing so in the future is roughly
the same as doing it now, so we took the call to punt it to future us.

We also decided to continue completely skipping InstallationTargets that
do not belong to a contender release (or that have CanOverride = false,
now), because that would slow down the installation controller too much
before #77 is closed. We captured that requirement in #110 though.
@osdrv osdrv added enhancement New feature or request good first issue Good for newcomers labels Jul 15, 2019
juliogreff added a commit that referenced this issue Jul 23, 2019
@juliogreff
Make InstallationTarget objects self contained
76828be 
Closes #5. You can read all of the context there too.

We want InstallationTarget objects to be self-contained, independent
from releases and applications. We're making them so by applying a few
changes:

1. The installation controller loses its smarts in regards to finding
out if the InstallationTarget is part of an incumbent or contender
release. This logic now belongs entiretly to the release controller.

2. The InstallationTarget becomes a richer object, by containing the
chart and values specified by the Application object at the time of the
creation of the release.

This commit takes a slightly different approach than the one described
in #5, though. Since we currently don't support installing different
things (either different charts or different values) per cluster with
the same InstallationTarget, we chose not to implement it here. That's
mostly in case YAGNI, and the cost of doing so in the future is roughly
the same as doing it now, so we took the call to punt it to future us.

We also decided to continue completely skipping InstallationTargets that
do not belong to a contender release (or that have CanOverride = false,
now), because that would slow down the installation controller too much
before #77 is closed. We captured that requirement in #110 though.
@osdrv osdrv added this to the release-0.7 milestone Aug 1, 2019
parhamdoustdar pushed a commit that referenced this issue Aug 28, 2019
@juliogreff
Make InstallationTarget objects self contained
3f4a1c0 
Closes #5. You can read all of the context there too.

We want InstallationTarget objects to be self-contained, independent
from releases and applications. We're making them so by applying a few
changes:

1. The installation controller loses its smarts in regards to finding
out if the InstallationTarget is part of an incumbent or contender
release. This logic now belongs entiretly to the release controller.

2. The InstallationTarget becomes a richer object, by containing the
chart and values specified by the Application object at the time of the
creation of the release.

This commit takes a slightly different approach than the one described
in #5, though. Since we currently don't support installing different
things (either different charts or different values) per cluster with
the same InstallationTarget, we chose not to implement it here. That's
mostly in case YAGNI, and the cost of doing so in the future is roughly
the same as doing it now, so we took the call to punt it to future us.

We also decided to continue completely skipping InstallationTargets that
do not belong to a contender release (or that have CanOverride = false,
now), because that would slow down the installation controller too much
before #77 is closed. We captured that requirement in #110 though.
@juliogreff juliogreff self-assigned this Sep 25, 2019
@juliogreff juliogreff mentioned this issue Sep 27, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@juliogreff juliogreff

Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
release-0.7
Development

Successfully merging a pull request may close this issue.

installation controller: reinstall deleted objects bookingcom/shipper
3 participants
@juliogreff @osdrv @parhamdoustdar