-
Notifications
You must be signed in to change notification settings - Fork 133
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
CONSOLE-2768: Update default and download deployments to use bindata #550
CONSOLE-2768: Update default and download deployments to use bindata #550
Conversation
1f272a9
to
a6d19df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering if we will need to rewrite the deployment_test.go since we are not assembling the deployment programatically.
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@florkbr are you still working on the PR? Or can the WIP be removed?
Found two naming nits.
Also wondering if we still need TestDefaultDeployment
and TestDefaultDownloadsDeployment
in the deployment_test.go
, since the the manifest is now static, except for generating:
- annotations
- replicas
- affinity
- volumes
- containers
Thinking of rather migrating these into helpers(if they are already not) and unit test rather those.
@jhadvig I will remove the WIP, but am in the process of making your code review comment changes. |
@florkbr let's keep |
a6d19df
to
beb49b6
Compare
@jhadvig this snowballed on me, so I'm adding back the WIP until we sync and confirm this is what you had in mind.
|
beb49b6
to
7d7eede
Compare
7d7eede
to
4dbbe9f
Compare
Migrates our inline manifests for the default console deployment and the download deployment to bindata while substituting in necessary variables. https://issues.redhat.com/browse/CONSOLE-2768
4dbbe9f
to
9953c8d
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: florkbr, jhadvig, RickJWagner The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/label docs-approved |
/hold cancel |
/label qe-approved |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
14 similar comments
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
Tech debt - migrates our inline manifests for the default console deployment and the download deployment to Go bindata while substituting in necessary variables. This will make it clearer where to modify the base deployments in the console operator (versus looking through Go code). See
bindata/deployments
in the diff.https://issues.redhat.com/browse/CONSOLE-2768