Skip to content

added registry console section per BZ#7331

Merged
mburke5678 merged 1 commit intoopenshift:masterfrom
mburke5678:BZ-1518204
Feb 7, 2018
Merged

added registry console section per BZ#7331
mburke5678 merged 1 commit intoopenshift:masterfrom
mburke5678:BZ-1518204

Conversation

@mburke5678
Copy link
Copy Markdown
Contributor

@mburke5678 mburke5678 commented Jan 25, 2018 •
edited
Loading

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 25, 2018
@mburke5678
Copy link
Copy Markdown
Contributor Author

mburke5678 commented Jan 29, 2018

@tripledes I am not clear on the openshift_cockpit_deployer_prefix setting. Can the prefix be any test string or does it point to an existing image and path?

@mburke5678
Copy link
Copy Markdown
Contributor Author

mburke5678 commented Jan 29, 2018

@tripledes PTAL

@tripledes
Copy link
Copy Markdown

tripledes commented Jan 29, 2018

@mburke5678 LGTM, @jtudelag works?

jtudelag
jtudelag reviewed Jan 30, 2018
View reviewed changes
Copy link
Copy Markdown

@jtudelag jtudelag left a comment

Choose a reason for hiding this comment

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

@mburke5678 @tripledes Please review ;)

Comment thread install_config/install/advanced_install.adoc Outdated
Copy link
Copy Markdown

@jtudelag jtudelag Jan 29, 2018

Choose a reason for hiding this comment

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

This example does not match the variable format <registry>/<namespace>/<cockpit-image-name>. The <cockpit-image-name> is missing... https://github.com/openshift/openshift-docs/pull/7331/files#diff-fe94157edddadf88425cf07b556a00a3R794

Shouldn't it be openshift_cockpit_deployer_prefix='registry.example.com/openshift/console instead?

@tripledes @mburke5678 please review!!

Copy link
Copy Markdown

@tripledes tripledes Jan 30, 2018

Choose a reason for hiding this comment

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

Based on https://github.com/openshift/openshift-ansible/blob/release-3.6/inventory/byo/hosts.ose.example#L416-L423 I'd say what @mburke5678 is correct. @mburke5678 could you please ping anyone from openshift-ansible to confirm?

Copy link
Copy Markdown
Contributor Author

@mburke5678 mburke5678 Jan 31, 2018

Choose a reason for hiding this comment

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

@sdodson Can you help? We are trying to determine an appropriate example for the openshift_cockpit_deployer_prefix parameter.
openshift_cockpit_deployer_prefix='registry.example.com/openshift'
openshift_cockpit_deployer_prefix='registry.example.com/openshift/console'
Other?

Copy link
Copy Markdown
Member

@sdodson sdodson Jan 31, 2018

Choose a reason for hiding this comment

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

If your image is at registry.example.com/openshift3/registry-console then it should be openshift_cockpit_deployer_prefix='registry.example.com/openshift3/' as registry-console is appended on to the end, though that too can be modified via openshift_cockpit_deployer_basename

@mburke5678
Copy link
Copy Markdown
Contributor Author

mburke5678 commented Jan 31, 2018 •
edited
Loading

@jianlinliu PTAL

jianlinliu
jianlinliu reviewed Feb 2, 2018
View reviewed changes
Comment thread install_config/install/advanced_install.adoc Outdated
Copy link
Copy Markdown

@jianlinliu jianlinliu Feb 2, 2018

Choose a reason for hiding this comment

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

This PR is targeted for 3.7, right? If yes, then it is okay. "openshift_cockpit_deployer_basename" is not supported yet in 3.6 openshift-ansible code yet.

Comment thread install_config/install/advanced_install.adoc Outdated
Copy link
Copy Markdown

@jianlinliu jianlinliu Feb 2, 2018

Choose a reason for hiding this comment

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

"registry-console" is the default value, in this example, it is better to change it to other than the default name

@mburke5678
Copy link
Copy Markdown
Contributor Author

mburke5678 commented Feb 5, 2018 •
edited
Loading

@tripledes Can you comment on @jianlinliu comment about which version that openshift_cockpit_deployer_basename was added? ^^
I removed openshift_cockpit_deployer_basename from this PR and do a follow-up to add the parameter to 3.7+, if needed.
See: #7578

@mburke5678 mburke5678 force-pushed the BZ-1518204 branch 2 times, most recently from 9ac309c to 43fe698 Compare February 5, 2018 17:33
@mburke5678
Copy link
Copy Markdown
Contributor Author

mburke5678 commented Feb 5, 2018 •
edited
Loading

[rev_history]
|xref:../install_config/install/advanced_install.adoc#install-config-install-advanced-install[Installing a Cluster -> Advanced Installation]
|Added information on xref:../install_config/install/advanced_install.adoc#advanced-install-configuring-registry-console[using a Cockpit registry console image other than the default].
%

@mburke5678
Copy link
Copy Markdown
Contributor Author

mburke5678 commented Feb 5, 2018

@openshift/team-documentation PTAL as we settle on versioning.

@mburke5678 mburke5678 mentioned this pull request Feb 5, 2018
Closed
@jianlinliu
Copy link
Copy Markdown

jianlinliu commented Feb 7, 2018

LGTM.

@mburke5678
Copy link
Copy Markdown
Contributor Author

mburke5678 commented Feb 7, 2018

@openshift/team-documentation PTAL

@ahardin-rh
Copy link
Copy Markdown
Contributor

ahardin-rh commented Feb 7, 2018

@mburke LGTM. In the revision history, you just need to update the first line to show:
[Installing a Cluster -> Advanced Installation] 🎆

@ahardin-rh ahardin-rh added the peer-review-done Signifies that the peer review team has reviewed this PR label Feb 7, 2018
@mburke5678 mburke5678 merged commit 91cf06f into openshift:master Feb 7, 2018
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Feb 7, 2018
@mburke5678
[enterprise-3.6] added registry console section per BZ
1805310 
(cherry picked from commit 07b256d) xref:openshift#7331
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Feb 7, 2018
@mburke5678
[enterprise-3.9] merge errors xref:openshift#7331
f59f65c
@mburke5678 mburke5678 mentioned this pull request Feb 7, 2018
Merged
@mburke5678 mburke5678 mentioned this pull request Feb 7, 2018
Merged
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Feb 7, 2018
@mburke5678
[enterprise-3.7] merge errors xref:openshift#7331
df31b99
mburke5678 added a commit to mburke5678/openshift-docs that referenced this pull request Feb 7, 2018
@mburke5678
[enterprise-3.7] added registry console section per BZ
3fd9515 
(cherry picked from commit 07b256d) xref:openshift#7331
@mburke5678 mburke5678 mentioned this pull request Feb 7, 2018
Merged
@mburke5678 mburke5678 deleted the BZ-1518204 branch February 15, 2018 20:23
vikram-redhat
vikram-redhat reviewed Feb 16, 2018
View reviewed changes
Comment thread install_config/install/advanced_install.adoc
openshift_docker_additional_registries=example.com
----

[[advanced-install-configuring-registry-location]]
Copy link
Copy Markdown
Contributor

@vikram-redhat vikram-redhat Feb 16, 2018

Choose a reason for hiding this comment

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

@mburke5678 this is a duplicate id, which seems to be copied from the previous section. I have changed it to advanced-install-configuring-registry-console.

@vikram-redhat
Copy link
Copy Markdown
Contributor

vikram-redhat commented Feb 16, 2018

@mburke5678 - the rev history link to the section id was not correct. Instead of configuring-a-registry-location it should have been advanced-install-configuring-registry-console.

@gaurav-nelson gaurav-nelson mentioned this pull request Feb 27, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sdodson sdodson sdodson left review comments

+4 more reviewers

@tripledes tripledes tripledes left review comments

@jianlinliu jianlinliu jianlinliu left review comments

@vikram-redhat vikram-redhat vikram-redhat left review comments

@jtudelag jtudelag jtudelag left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

branch/enterprise-3.6 branch/enterprise-3.7 branch/enterprise-3.9 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@mburke5678 @tripledes @jianlinliu @ahardin-rh @vikram-redhat @sdodson @jtudelag @openshift-ci-robot