Skip to content
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

OSDOCS:2475 Adds new assembly for the dynamic plugin TP #41260

Merged
merged 1 commit into from
Feb 28, 2022

Conversation

opayne1
Copy link
Contributor

@opayne1 opayne1 commented Feb 1, 2022

New assembly and modules for the Dynamic Plugin Tech Preview
Relates to: OSDOCS-2475, CONSOLE-2907, and CONSOLE-3036

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 1, 2022
@netlify
Copy link

netlify bot commented Feb 1, 2022

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 7dd13ea

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/621cf792fc5a1f00081664da

😎 Browse the preview: https://deploy-preview-41260--osdocs.netlify.app

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 1, 2022
@opayne1 opayne1 force-pushed the OSDOCS-2475-pt-2 branch 2 times, most recently from bcae31b to 4b02dc3 Compare February 2, 2022 18:29
@openshift-ci openshift-ci bot 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 Feb 2, 2022
@opayne1 opayne1 force-pushed the OSDOCS-2475-pt-2 branch 11 times, most recently from 325006f to 2de08e2 Compare February 9, 2022 17:04
@TheRealJon
Copy link
Member

In order to get started using the dynamic plugin, the template containing a Dockerfile for building plugin images and an OpenShift Container Platform template for adding your plugin to the cluster.

I'm not sure what this sentence means. Looks like maybe something was omitted or missed in editing.

@TheRealJon
Copy link
Member

IMO, the first step under "Procedure" should be more descriptive about where the "Use this template" button appears.

Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

Thanks, @opayne1. I know this is still WIP, but wanted to give you some feedback.

modules/about-dynamic-plugin.adoc Outdated Show resolved Hide resolved
modules/about-dynamic-plugin.adoc Outdated Show resolved Hide resolved
modules/about-dynamic-plugin.adoc Outdated Show resolved Hide resolved
modules/about-dynamic-plugin.adoc Outdated Show resolved Hide resolved
modules/deployment-plugin-cluster.adoc Outdated Show resolved Hide resolved
modules/dynamic-plugins-get-started.adoc Outdated Show resolved Hide resolved
modules/dynamic-plugins-get-started.adoc Outdated Show resolved Hide resolved
modules/running-your-dynamic-plugin.adoc Outdated Show resolved Hide resolved
modules/running-your-dynamic-plugin.adoc Outdated Show resolved Hide resolved
_topic_maps/_topic_map.yml Outdated Show resolved Hide resolved
@spadgett
Copy link
Member

@christianvogt @vojtechszocs FYI

@spadgett
Copy link
Member

cc @alimobrem

@opayne1 opayne1 force-pushed the OSDOCS-2475-pt-2 branch 2 times, most recently from 13c072f to 9d8e1d6 Compare February 21, 2022 20:15
@michaelryanpeter michaelryanpeter added branch/enterprise-4.10 peer-review-needed Signifies that the peer review team needs to review this PR labels Feb 24, 2022
@michaelryanpeter michaelryanpeter added this to the Future Release milestone Feb 24, 2022
@opayne1 opayne1 force-pushed the OSDOCS-2475-pt-2 branch 2 times, most recently from a7db00c to 1d0c7d7 Compare February 24, 2022 20:59
Copy link
Contributor

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

Looking really great. Just some small notes.

modules/adding-new-extension-dynamic-plug-in.adoc Outdated Show resolved Hide resolved
modules/adding-new-extension-dynamic-plug-in.adoc Outdated Show resolved Hide resolved
modules/running-your-dynamic-plug-in.adoc Outdated Show resolved Hide resolved
[id="dynamic-plugin-additional-resources"]
== Additional resources

* The console customization plug-in can provide some guidance to building a dynamic plug-in, refer to link:https://github.com/spadgett/console-customization-plugin[Console customization plug-in].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* The console customization plug-in can provide some guidance to building a dynamic plug-in, refer to link:https://github.com/spadgett/console-customization-plugin[Console customization plug-in].
* link:https://github.com/spadgett/console-customization-plugin[Console customization plug-in].

I think this is the new format required for additional resources in Jupiter. I can ask in the team chat for verification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made the change. I will verify this.

modules/about-dynamic-plug-ins.adoc Outdated Show resolved Hide resolved
modules/about-dynamic-plug-ins.adoc Outdated Show resolved Hide resolved
modules/build-image-docker.adoc Outdated Show resolved Hide resolved
modules/build-image-docker.adoc Outdated Show resolved Hide resolved
modules/dynamic-plug-ins-get-started.adoc Outdated Show resolved Hide resolved
@opayne1
Copy link
Contributor Author

opayne1 commented Feb 24, 2022

@yapei I have made some changes. I think this is ready for your eyes again. Thank you!

@spadgett
Copy link
Member

I'm running on Fedora and Docker is expected to be used, right?

@yapei The script will use podman first if installed, then fall back to Docker. I might change this, but we should figure out what's wrong.

https://github.com/spadgett/console-plugin-template/blob/main/start-console.sh#L26-L32

I don't know that we should hold the doc since it's a bug in the template repo script and not the documentation.

However I didn't see an entry host.containers.internal in /etc/hosts file, anything wrong?

I believe it's in the /etc/hosts file inside the container, not on the host system.

@spadgett
Copy link
Member

@yapei I pushed a fix to the template repo. Can you try again with the latest?

@opayne1
Copy link
Contributor Author

opayne1 commented Feb 25, 2022

@spadgett @yapei are docs good to merge?

Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

@opayne1 Looks good to me!

@yapei
Copy link

yapei commented Feb 28, 2022

@yapei I pushed a fix to the template repo. Can you try again with the latest?

Yeah, it works! Thanks!

@yapei
Copy link

yapei commented Feb 28, 2022

it looks good to me now ~

Copy link
Contributor

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

Just a few things I missed last time. After these updates, we can merge.

modules/about-dynamic-plug-ins.adoc Outdated Show resolved Hide resolved
modules/deployment-plug-in-cluster.adoc Outdated Show resolved Hide resolved
modules/adding-tab-pods-page.adoc Outdated Show resolved Hide resolved
modules/adding-tab-pods-page.adoc Outdated Show resolved Hide resolved
web_console/dynamic-plug-ins.adoc Outdated Show resolved Hide resolved
@michaelryanpeter michaelryanpeter 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 Feb 28, 2022
@opayne1 opayne1 changed the title [WIP] OSDOCS:2475 Adds new assembly for the dynamic plugin TP OSDOCS:2475 Adds new assembly for the dynamic plugin TP Feb 28, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 28, 2022
@michaelryanpeter michaelryanpeter merged commit e591b00 into openshift:main Feb 28, 2022
@michaelryanpeter
Copy link
Contributor

/cherrypick enterprise-4.10

@openshift-cherrypick-robot

@michaelryanpeter: new pull request created: #42512

In response to this:

/cherrypick enterprise-4.10

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.

@bobfuru bobfuru modified the milestones: Future Release, OCP 4.10 GA Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.10 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