Skip to content

Refactor remaining deployment controllers for retry support #1170

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.

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
merged 1 commit into from
Feb 27, 2015

Conversation

ironcladlou
Copy link
Contributor

Followup to #1091 in support of #824.

Summary of the changes:

  • Each controller is now in its own package (pkg/deploy/controller/$type) with a normalized name (controller.go, factory.go, etc.)
  • DeploymentController has been split into DeploymentController and DeployerPodController
  • All controllers now propagate semantic errors (e.g. fatal or nonfatal)
  • Improved private interface usage throughout the controllers
  • All controllers are now wrapped in a RetryController via their corresponding factories

@ironcladlou
Copy link
Contributor Author

I broke this down into bite-sized commits for you, @pmorie.

@ironcladlou
Copy link
Contributor Author

[test]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_openshift3/1204/)

@ironcladlou ironcladlou force-pushed the deploy-retry-refactor branch from a4bea20 to 5e241ac Compare February 26, 2015 22:38
@ironcladlou
Copy link
Contributor Author

@bparees might be interested in this for builds.

@pmorie
Copy link
Contributor

pmorie commented Feb 27, 2015

Provisional LGTM, but I need to take a more detailed look at the tests.

@@ -12,15 +12,17 @@ import (
)

// Test the controller's response to a new DeploymentConfig with a config change trigger.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this comment is no longer correct

@ironcladlou ironcladlou force-pushed the deploy-retry-refactor branch 2 times, most recently from c7b9f6c to d3fc43b Compare February 27, 2015 21:31
Followup to openshift#1091 in support of openshift#824.

Summary of the changes:

* Each controller is now in its own package (`pkg/deploy/controller/$type`) with a normalized name (`controller.go`, `factory.go`, etc.)
* `DeploymentController` has been split into `DeploymentController` and `DeployerPodController`
* All controllers now propagate semantic errors (e.g. fatal or nonfatal)
* Improved private interface usage throughout the controllers
* All controllers are now wrapped in a `RetryController` via their corresponding factories
@ironcladlou ironcladlou force-pushed the deploy-retry-refactor branch from d3fc43b to 224b3b4 Compare February 27, 2015 21:35
@pmorie
Copy link
Contributor

pmorie commented Feb 27, 2015

LGTM [merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1072/) (Image: devenv-fedora_929)

@openshift-bot
Copy link
Contributor

Evaluated for origin up to 224b3b4

openshift-bot pushed a commit that referenced this pull request Feb 27, 2015
@openshift-bot openshift-bot merged commit 6fbae4e into openshift:master Feb 27, 2015
@ironcladlou ironcladlou deleted the deploy-retry-refactor branch March 2, 2015 14:42
jpeeler pushed a commit to jpeeler/origin that referenced this pull request Sep 5, 2017
…service-catalog/' changes from 7e650e7e39..ef63307bdb

ef63307bdb origin build: add origin tooling
a876fe3 v0.0.17 (openshift#1178)
c5237fe correct osbapi service definition (openshift#1177)
6036d4e Adding walkthrough instructions for 1.7 (openshift#1171)
5f111dd Specifying that you need Helm v2.5.0 for installation (openshift#1170)
08043bd Adding more small fixes to the walkthrough & install docs (openshift#1169)
d65d4a1 rbac targets needed to be renamed as well (openshift#1161)
590f6f2 Write helm command to file for api aggregation (openshift#1141)
49ddcf6 clean before building a specific arch (openshift#1168)
43f7cfb Splitting up the Walkthrough for 1.6 and 1.7 instructions (openshift#1163)
02e0217 Updates to README (openshift#1166)
57f2aa5 Adding instructions for installing from Macs (openshift#1164)
dfe620e fix rate-limiting for polling queue (openshift#1143)
ca5f335 Use Generation instead of checksum for Broker (openshift#1145)
5364daa Merge branch 'pr/1158'
f34c5db move Travis deployment script to directory in 'contrib/'
2a00d7f Update incorrect port (openshift#1156)
b0ed60e improve the repository's layout (openshift#1154)
f870baf Follow up file / renames from openshift#1142 (openshift#1152)
826b4f9 remove unnecessary json annotations (openshift#1153)
33cb345 Rename resources. closes openshift#1080 (openshift#1142)
70c2b9b Add ability to specify CA certs to use for TLS authentication. (openshift#1112)
2aa5039 v0.0.16 (openshift#1140)
65de49c Comments for unit test bullet proofing (openshift#1139)
REVERT: 7e650e7e39 origin build: add origin tooling

git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog
git-subtree-split: ef63307bdbaa64efca204912f5361a4f3d3be2c8
jpeeler pushed a commit to jpeeler/origin that referenced this pull request Sep 11, 2017
…service-catalog/' changes from 7e650e7e39..ef63307bdb

ef63307bdb origin build: add origin tooling
a876fe3 v0.0.17 (openshift#1178)
c5237fe correct osbapi service definition (openshift#1177)
6036d4e Adding walkthrough instructions for 1.7 (openshift#1171)
5f111dd Specifying that you need Helm v2.5.0 for installation (openshift#1170)
08043bd Adding more small fixes to the walkthrough & install docs (openshift#1169)
d65d4a1 rbac targets needed to be renamed as well (openshift#1161)
590f6f2 Write helm command to file for api aggregation (openshift#1141)
49ddcf6 clean before building a specific arch (openshift#1168)
43f7cfb Splitting up the Walkthrough for 1.6 and 1.7 instructions (openshift#1163)
02e0217 Updates to README (openshift#1166)
57f2aa5 Adding instructions for installing from Macs (openshift#1164)
dfe620e fix rate-limiting for polling queue (openshift#1143)
ca5f335 Use Generation instead of checksum for Broker (openshift#1145)
5364daa Merge branch 'pr/1158'
f34c5db move Travis deployment script to directory in 'contrib/'
2a00d7f Update incorrect port (openshift#1156)
b0ed60e improve the repository's layout (openshift#1154)
f870baf Follow up file / renames from openshift#1142 (openshift#1152)
826b4f9 remove unnecessary json annotations (openshift#1153)
33cb345 Rename resources. closes openshift#1080 (openshift#1142)
70c2b9b Add ability to specify CA certs to use for TLS authentication. (openshift#1112)
2aa5039 v0.0.16 (openshift#1140)
65de49c Comments for unit test bullet proofing (openshift#1139)
REVERT: 7e650e7e39 origin build: add origin tooling

git-subtree-dir: cmd/service-catalog/go/src/github.com/kubernetes-incubator/service-catalog
git-subtree-split: ef63307bdbaa64efca204912f5361a4f3d3be2c8
jpeeler pushed a commit to jpeeler/origin that referenced this pull request Feb 1, 2018
* Specifying that you need Helm v2.5.0 for installation

This is a follow-up to
kubernetes-retired/service-catalog#1163 (comment)
sion_r135920291

Cc/ @MHBauer

* Reducing Helm version redundancy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants