Skip to content

osdocs-154 Document Registry Operator#14476

Merged
bmcelvee merged 1 commit intoopenshift:enterprise-4.1from
bmcelvee:osdocs-154-registry-operator
May 10, 2019
Merged

osdocs-154 Document Registry Operator#14476
bmcelvee merged 1 commit intoopenshift:enterprise-4.1from
bmcelvee:osdocs-154-registry-operator

Conversation

@bmcelvee
Copy link
Copy Markdown
Contributor

@bmcelvee bmcelvee added this to the Future Release milestone Apr 16, 2019
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 16, 2019
@bmcelvee bmcelvee changed the title osdocs-154 Document Registry Operator [WIP] osdocs-154 Document Registry Operator Apr 16, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 16, 2019
@bmcelvee bmcelvee changed the base branch from enterprise-4.0 to enterprise-4.1 April 17, 2019 14:25
@bmcelvee bmcelvee force-pushed the osdocs-154-registry-operator branch from 3e3122c to 5901e4d Compare April 30, 2019 20:00
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 30, 2019
@bmcelvee bmcelvee force-pushed the osdocs-154-registry-operator branch from 5901e4d to 2004380 Compare May 1, 2019 15:01
@bmcelvee
Copy link
Copy Markdown
Contributor Author

bmcelvee commented May 1, 2019

@dmage PTAL, thanks! :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

configs.imageregistry.operator.openshift.io/cluster

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is implemented.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

config.imageregistry.operator.openshift.io?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should be configs.imageregistry.operator.openshift.io

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This can't be changed by a user anymore. They should create config map in the openshift-config namespace and use AdditionalTrustedCA in the image.config.openshift.io resource.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @dmage! I temporarily commented out the original config content, and put in the following:

"You can create a ConfigMap in the openshift-config namespace and use AdditionalTrustedCA in the image.config.openshift.io resource to contact external registries."

Does that make sense? I think it would also be helpful to provide an example.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@bmcelvee

"You can create a ConfigMap in the openshift-config namespace and use it name in AdditionalTrustedCA in the image.config.openshift.io resource to provide additional certificate authorities that should be trusted when contacting external registries."

Something like this?.. Maybe it should be a link for another document. For the registry the keys in this ConfigMap don't matter. But for runtime and builds, the config map should look like this:

apiVersion: v1
kind: ConfigMap
metadata:
  name: my-registry-ca
data:
  registry.example.com: |
    -----BEGIN CERTIFICATE-----
    ...
    -----END CERTIFICATE-----
  registry-with-port.example.com..5000: |
    -----BEGIN CERTIFICATE-----
    ...
    -----END CERTIFICATE-----

The key is the host name of a registry with the port for which this CA is to be trusted. Note that if the registry has the port, like registry-with-port.example.com:5000, : should be replaced with ...

@adambkaplan are there any additional requirements for this from builds?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dmage @bmcelvee worth mentioning that the certIificates must be PEM-encoded (which is the norm). I would stick to the format needed by builds and runtimes in any example.

@bmcelvee bmcelvee force-pushed the osdocs-154-registry-operator branch from 2004380 to 9fd2ad8 Compare May 2, 2019 18:39
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dmage - another question: Is there a default config.imageregistry.operator.openshift.io CRD?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@bmcelvee yes, the image registry operator (in managed state) always creates configs.imageregistry.operator.openshift.io/cluster when it doesn't exist.

@bmcelvee
Copy link
Copy Markdown
Contributor Author

bmcelvee commented May 2, 2019

Thanks, @dmage! Just have a couple more questions on this one.

@bmcelvee bmcelvee force-pushed the osdocs-154-registry-operator branch 4 times, most recently from 94c9aa7 to c59fc3b Compare May 3, 2019 18:52
@bmcelvee
Copy link
Copy Markdown
Contributor Author

bmcelvee commented May 3, 2019

@wzheng1 PTAL, thanks!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It is better to highlight some configures are needed during installation for UPI on baremetal and vsphere, since there is no default storage configure for them, it will return: Unable to apply resources: storage backend not configured.
In this case, when try to describe clusteropeartor image-registry:
Status:
Conditions:
Last Transition Time: 2019-04-15T05:28:10Z
Message: The registry is ready
Reason: Ready
Status: True
Type: Available
Last Transition Time: 2019-04-16T05:42:07Z
Message: Unable to apply resources: storage backend not configured
Reason: Error
Status: True
Type: Progressing
Last Transition Time: 2019-04-16T05:42:07Z
Message: storage backend not configured
Reason: StorageNotConfigured
Status: True
Type: Failing

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @wzheng1! I added the following note, and am still working on the registry storage PR to go in the "Installing" book - #14390.

"Storage is only automatically configured when you install on AWS."

@bmcelvee bmcelvee force-pushed the osdocs-154-registry-operator branch from c59fc3b to 4d70646 Compare May 6, 2019 15:13
@bmcelvee
Copy link
Copy Markdown
Contributor Author

bmcelvee commented May 8, 2019

@openshift/team-documentation please peer review, thanks!

@bmcelvee bmcelvee changed the title [WIP] osdocs-154 Document Registry Operator osdocs-154 Document Registry Operator May 8, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 8, 2019
@bmcelvee bmcelvee added the peer-review-needed Signifies that the peer review team needs to review this PR label May 10, 2019
@bmcelvee bmcelvee force-pushed the osdocs-154-registry-operator branch from 4d70646 to 48f2ba6 Compare May 10, 2019 18:10
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

`image-registry` ?
Are these UI fields or parameters in the yaml definition? If they're in the yaml, they should be "configuration parameters."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We switched to `` instead of ** for parameters and values in 4.1.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/operator/Operator
^ all places

Please double-check that all the future tense is necessary - present tense often is good enough.
The : should be outside the formatting.
Double-check that each entry starts with a capital letter and ends with a period.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are these fields, or components, or states? It feels like the descriptions have different functions for different entries. I feel like some of them would benefit from having values or default values provided.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This paragraph sounds like it should be a procedure module.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd combine this instruction with the step.
Also, why am I patching it, and what am I patching it with?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe s/. It/, and it
registry,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd s/AWS/Amazon Web Services

Do you have an issue to add, or refer to, the image registry storage configuration modules when they exist?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"On initial startup" or "After the control plane deploys" ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Like if you don't have storage set up?

@kalexand-rh kalexand-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels May 10, 2019
@bmcelvee bmcelvee force-pushed the osdocs-154-registry-operator branch from 48f2ba6 to d3a66cb Compare May 10, 2019 19:20
@bmcelvee
Copy link
Copy Markdown
Contributor Author

Going ahead and merging. Any additional updates can be made in follow-up PRs.

@bmcelvee bmcelvee merged commit f147cc6 into openshift:enterprise-4.1 May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.1 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants