-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Add OLM disconnected config #16458
Add OLM disconnected config #16458
Conversation
5796e9d
to
8f29b00
Compare
@ecordell @shawn-hurley FYI on this WIP. |
8f29b00
to
e8db3e3
Compare
18c07f5
to
5998ab8
Compare
b4ab0bf
to
a692f1e
Compare
@openshift/team-documentation PTAL for peer review. |
CMD ["--database", "bundles.db"] | ||
---- | ||
|
||
.. Use the `podman` command to create and tag the container image from the |
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.
Do we really want customers building an image?
Shouldn't we be providing this image to customers?
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.
I have no idea who would be responsible for building/pushing and managing these images.
community-operators and certified-operators and redhat-operators are expecting changes, often by individual teams. I don't know how we keep this image up to date or how to tie this to an errata/release process.
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.
@sosiouxme maybe we can chat about this offline?
└─ <CSVs_and_CRDs_and_a_package_file> | ||
---- | ||
|
||
... If you see files already in the structure described in the previous step, then you do not have to do anything. |
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.
Shouldn't they be in this format (based on the instructions we are giving in the piro step?
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.
No, there are two different things in the tar potentially. one that is just bundle.yaml
and one that is broken up already. This is stating that if you don't see the bundle.yaml then you don't need to do anything. Can we make that more clear? Does that make sense?
$ curl https://quay.io/cnr/api/v1/packages?namespace=certified-operators >> packages.txt | ||
---- | ||
+ | ||
Each package in the new `packages.txt` is an Operator that you could add to your |
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.
Can we explain to customers how to parse this?
Can we provide them a bash script that does all this?
a083dfe
to
dbf5d5a
Compare
@jianzhangbjz @bandrade @scolange PTAL for QE review. |
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.
Looks good! A couple nitpicks.
$ curl -XGET https://quay.io/cnr/api/v1/packages/community-operators/etcd/blobs/sha256/8108475ee5e83a0187d6d0a729451ef1ce6d34c44a868a200151c36f3232822b -o etcd.tar.gz | ||
---- | ||
|
||
.. To pull the information out, you must untar the archive into a directory with |
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.
Instead of "pull the information out", what about "To extract the information"?
Same with line 70 above: "use it to extract the gzipped archive"
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.
@adellape Just added a suggestion but LGTM for me.
|
||
.. In this directory, you must now handle two different types of bundles: | ||
|
||
... If you see a single file called `bundle.yaml`, you must break the content under |
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.
I'm really concerned about letting the user do this step, but I know that are improvements being implemented. Is it possible to add the bundle.yaml in this doc used for this example? That would help the user to associate with their use cases.
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.
@bandrade I've added step 4 about how to break up the file to http://file.rdu.redhat.com/~adellape/083019/olm_disconnected/applications/operators/olm-disconnected.html.
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.
Thanks @adellape, lgmt.
8f40a6d
to
23abd99
Compare
7f5aaa3
to
fad25c3
Compare
apiVersion: config.openshift.io/v1 | ||
kind: OperatorHub | ||
spec: | ||
disableAllDefaultSources: true |
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.
Nit, you can set this with a copy/pastable command like:
$ oc patch OperatorHub cluster --type json -p '[{"op": "add", "path": "/spec/disableAllDefaultSources", "value": true}]'
to avoid dropping into an editor.
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.
👍 Updated.
fad25c3
to
7f0a967
Compare
7f0a967
to
903e5f6
Compare
/cherrypick enterprise-4.2 |
@adellape: new pull request created: #16866 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
+ | ||
---- | ||
$ oc image mirror quay.io/coreos/etcd-operator \ | ||
<internal_registry_route>:5000/coreos/etcd-operator |
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.
Parameterize this port? e.g. we use <local_registry_host_name>:<local_registry_host_port>
in a number of other places in the enterprise-4.2 branch (CC @kalexand-rh).
@@ -511,6 +511,9 @@ Topics: | |||
- Name: Creating policy for Operator installations and upgrades | |||
File: olm-creating-policy | |||
Distros: openshift-origin,openshift-enterprise | |||
- Name: Configuring OLM in disconnected mode |
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.
The release mirroring PRs went with "restricted network" instead of "disconnected" (e.g. see #16696).
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.
@vikram-redhat please re-review this PR and look at the denoted comments and consider them for inclusion.
$ mkdir -p manifests/etcd/ <1> | ||
$ tar -xf etcd.tar.gz -C manifests/etcd/ | ||
---- | ||
<1> Create different subdirectories for each extracted archive so that files are not |
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.
I think this needs to be a call out or warning.
@adellape can you consider modifying this?
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.
A <#>
is considered a "callout icon" in AsciiDoc, and I think in this case it's better to keep the connection / context using the numbered icon on the line item (switching to a Warning box would lose that and doesn't seem to add much).
Dockerfile: | ||
+ | ||
---- | ||
$ podman build -f custom-registry.Dockerfile \ |
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.
@adellape we need to add a note about setting up podman (to authenticate with the customer portal).
Without this, the build will fail because of the registry being used.
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.
Since we can't link to modules from within a module, I had to add the link to the registry login procedure in the "Additional resources" at the bottom of the assembly:
disconnected {product-title} cluster | ||
+ | ||
---- | ||
$ podman push <internal_registry_route>/<namespace>/custom-registry |
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.
Should we specify what namespace to push this too?
What authentication needs to be provided to podman for this push to work?
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.
@ecordell @shawn-hurley My understanding was it could be any namespace. Should we state one? Existing or have them create a new one?
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.
This is the same registry and info that is in the main disconnected doc correct? we should just follow that IMO.
└── package.yaml | ||
---- | ||
+ | ||
If you see files already in this structure, you can skip this step. However, if |
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.
@adellape I think we also need to tell the user to record or pull out the image that is used by the operator versions. We need this for step 8.
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.
Added as a new step 5 in #16892.
You should also be able to view them from the *OperatorHub* page in the web | ||
console. | ||
|
||
. **Mirror the images required by the Operators you want to use.** |
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.
In steps 3 and/or 4 you need to tell the user to pull out or record the image path/url. It is needed in this step.
LGTM from QE received in the followup PR: #16892. Removing QE required flag. |
xref: https://jira.coreos.com/browse/OSDOCS-658
Preview (internal): http://file.rdu.redhat.com/~adellape/083019/olm_disconnected/applications/operators/olm-disconnected.html