Skip to content

Remove the AssetConfig #18068

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 3 commits into from
Jan 23, 2018
Merged

Remove the AssetConfig #18068

openshift-merge-robot merged 3 commits into from
Jan 23, 2018

Conversation

@deads2k
Copy link
Contributor

@deads2k deads2k commented Jan 11, 2018

Remove the AssetConfig

/assign spadgett
/assign mfojtik

@juanvallejo

@openshift-ci-robot openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 11, 2018
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 11, 2018
@spadgett
Copy link
Member

spadgett commented Jan 11, 2018

@sdodson We should be able to remove assetConfig from the openshift-ansible master-config.yaml template when this merges.

@jwforres fyi

@bparees bparees removed their request for review January 11, 2018 16:24
@deads2k deads2k force-pushed the api-16-remove-asset branch 2 times, most recently from da5fce1 to f63b4df Compare January 12, 2018 15:41
@soltysh
Copy link
Contributor

soltysh commented Jan 15, 2018

@sdodson We should be able to remove assetConfig from the openshift-ansible master-config.yaml template when this merges.

But it'll be migrated to a separate one? So that the webconsole is properly installed, right?

soltysh
soltysh approved these changes Jan 15, 2018
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 16, 2018
@sdodson
Copy link
Member

sdodson commented Jan 16, 2018

But it'll be migrated to a separate one? So that the webconsole is properly installed, right?

It gets migrated to a configmap in openshift-web-console namespace.

@deads2k deads2k force-pushed the api-16-remove-asset branch from f63b4df to 6c7c526 Compare January 16, 2018 18:36
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 16, 2018
@deads2k deads2k added the lgtm Indicates that a PR is ready to be merged. label Jan 16, 2018
@deads2k
Copy link
Contributor Author

deads2k commented Jan 16, 2018

@smarterclayton I need a new openshift tag to land this.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 17, 2018

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 17, 2018

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 17, 2018

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 18, 2018

/retest

Please review the full test history for this PR and help us cut down flakes.

@deads2k
Copy link
Contributor Author

deads2k commented Jan 18, 2018

/retest

@deads2k deads2k force-pushed the api-16-remove-asset branch from 6c7c526 to 8a936d1 Compare January 18, 2018 17:09
@deads2k deads2k removed the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2018
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jan 18, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@deads2k deads2k added the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2018
@deads2k
Copy link
Contributor Author

deads2k commented Jan 18, 2018

larger hammer acquired.

@smarterclayton after this one merges, I'll need yet another tag.

@deads2k
Copy link
Contributor Author

deads2k commented Jan 18, 2018

/retest

@deads2k deads2k force-pushed the api-16-remove-asset branch from 8a936d1 to f60bcf0 Compare January 18, 2018 18:00
@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2018
spadgett
spadgett reviewed Jan 18, 2018
View reviewed changes
pkg/oc/bootstrap/docker/openshift/helper.go
cfg.AssetConfig = &configapi.AssetConfig{}
}
cfg.AssetConfig.ExtensionScripts = append(cfg.AssetConfig.ExtensionScripts, serviceCatalogExtensionPath)

Copy link
Member

@spadgett spadgett Jan 18, 2018

Choose a reason for hiding this comment

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

Lines for extensions below this should also be removed (839-849)

@spadgett spadgett mentioned this pull request Jan 23, 2018
Merged
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 23, 2018
@deads2k deads2k force-pushed the api-16-remove-asset branch from 4d963a8 to 3c58eec Compare January 23, 2018 16:37
@deads2k deads2k added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2018
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 23, 2018
@openshift-merge-robot
Copy link
Contributor

openshift-merge-robot commented Jan 23, 2018

Automatic merge from submit-queue (batch tested with PRs 18233, 18068, 18228, 18227).

@openshift-merge-robot openshift-merge-robot merged commit 4418e6b into openshift:master Jan 23, 2018
@spadgett spadgett mentioned this pull request Jan 24, 2018
Merged
@deads2k deads2k deleted the api-16-remove-asset branch January 24, 2018 14:28
openshift-merge-robot added a commit that referenced this pull request Jan 25, 2018
@openshift-merge-robot
Merge pull request #18264 from spadgett/remove-cluster-up-extension
074b1a7 
Automatic merge from submit-queue (batch tested with PRs 18191, 18264, 18235, 18251, 18271).

Remove obsolete catalog extension file from cluster up

Follow on to #18068. This extension file is no longer needed for the console to detect TSB is running.

/assign @bparees 
/cc @deads2k @jwforres
@sjenning sjenning mentioned this pull request Jan 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spadgett spadgett spadgett left review comments

@gabemontero gabemontero Awaiting requested review from gabemontero

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

+2 more reviewers

@soltysh soltysh soltysh approved these changes

@juanvallejo juanvallejo juanvallejo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mfojtik mfojtik

@spadgett spadgett

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. needs-api-review size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@deads2k @spadgett @soltysh @sdodson @openshift-bot @openshift-ci-robot @openshift-merge-robot @juanvallejo @mfojtik