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

OSSMDOC-550: Document new WasmPlugin API. #45111

Merged
merged 1 commit into from Jun 23, 2022

Conversation

JStickler
Copy link
Contributor

@JStickler JStickler commented Apr 28, 2022

OSSMDOC-550: Document new WasmPlugin API.

OCP versions 4.6 - 4.11

This PR:

  • Breaks the existing extensions assembly into ServiceMeshExtension modules
  • Adds modules for the new WasmPlugin extension

Link to final docs preview: http://file.bos.redhat.com/jstickle/OSSMDOC-550/service_mesh/v2x/ossm-extensions.html

Link to docs preview: https://deploy-preview-45111--osdocs.netlify.app/openshift-enterprise/latest/service_mesh/v2x/ossm-extensions

Eng review - dgn, jwendell
QE review - skondkar
Peer review - jldohmann

@netlify
Copy link

netlify bot commented Apr 28, 2022

Deploy Preview for osdocs ready!

Name Link
🔨 Latest commit ad5b853bf7a85e3b0ad5474c441f1bbf15b05273
🔍 Latest deploy log https://app.netlify.com/sites/osdocs/deploys/6282a9d105afa10008ff31c1
😎 Deploy Preview https://deploy-preview-45111--osdocs.netlify.app/openshift-enterprise/latest/service_mesh/v2x/ossm-extensions
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@JStickler JStickler force-pushed the OSSMDOC-550 branch 2 times, most recently from b54558d to 2d55335 Compare April 28, 2022 19:15
@JStickler
Copy link
Contributor Author

@dgn, please review since you are the resident expert on WASM.

Copy link

@dgn dgn left a comment

Choose a reason for hiding this comment

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

lgtm except for the container image format

modules/ossm-extensions-ref-wasmplugin.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-container-format.adoc Outdated Show resolved Hide resolved
@JStickler JStickler force-pushed the OSSMDOC-550 branch 2 times, most recently from 29d8921 to 853bf9b Compare May 12, 2022 18:31
@JStickler
Copy link
Contributor Author

@dgn I revised the WasmPlugin container format topic. I could use a second review for that topic please.

modules/ossm-extensions-overview.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-overview.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-wasmplugin-format.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-wasmplugin-format.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-ref-wasmplugin.adoc Show resolved Hide resolved
modules/ossm-extensions-ref-wasmplugin.adoc Show resolved Hide resolved
modules/ossm-extensions-ref-wasmplugin.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-wasmplugin-deploy.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-wasmplugin-deploy.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-ref-wasmplugin.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-ref-wasmplugin.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-wasmplugin-deploy.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-ref-smextension.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-ref-smextension.adoc Show resolved Hide resolved
@skondkar
Copy link

skondkar commented Jun 1, 2022

Verified with the steps in preview doc. Created WasmPlugin for OSSM 2.2

When creating ServiceMeshExtenstion on 2.2, got the error for unrecognized type ServiceMeshExtension
Successfully created ServiceMeshExtenstion on 2.1 as per the example in the preview doc

Screenshot from 2022-06-01 16-19-57
Screenshot from 2022-06-01 16-28-25

@jwendell
Copy link
Member

jwendell commented Jun 1, 2022

@skondkar Can you provide more details on the error on 2.2? Feel free to create a ticket in Jira with the details/step to reproduce and tag me there.

@skondkar
Copy link

skondkar commented Jun 1, 2022

About error on 2.2, the steps followed:

apiVersion: maistra.io/v1
kind: ServiceMeshExtension
metadata:
name: header-append
namespace: istio-system
spec:
workloadSelector:
labels:
app: httpbin
config:
first-header: some-value
another-header: another-value
image: quay.io/maistra-dev/header-append-filter:2.1
phase: PostAuthZ
priority: 100

  • It shows below error after applying the file:

Error from server: error when creating "extension.yaml": admission webhook "rev.validation.istio.io" denied the request: unrecognized type ServiceMeshExtension

Screenshot-Error-ServiceMeshExtension

  • It works if i install v2.1 SMCP and apply the same yaml file

@jwendell
Copy link
Member

jwendell commented Jun 1, 2022

Please, create a Jira and include all details in there (OCP version, SMCP yaml, etc)

@skondkar
Copy link

skondkar commented Jun 1, 2022

@jwendell thanks,
Created the issue to track: https://issues.redhat.com/browse/OSSM-1597

@skondkar
Copy link

skondkar commented Jun 3, 2022

LGTM.
Tested the PR again after verifying issue [OSSM-1597] on OSSM 2.2 latest build.
Created WasmPlugin resources and ServiceMeshExtension resource successfully on SMCP v2.2 as per the preview doc.

Verified that while creating ServiceMeshExtension, it shows below warning:
Warning: ServiceMeshExtension is deprecated. It works now but will be removed in a future version. Please use WasmPlugin instead.

@JStickler
Copy link
Contributor Author

@jwendell @skondkar if I'm reading the comments on this correctly, now that we have a user friendly message about the ServiceMeshExtension, am I good to move this PR to peer review? Or is there something else I need to update?

@JStickler
Copy link
Contributor Author

Received ack via Slack that this PR is good to go and can move to peer review.

Copy link
Contributor

@jldohmann jldohmann left a comment

Choose a reason for hiding this comment

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

Looking good! Mostly some minor ISG nits and API object references. I realize that a lot of what GH perceives as file changes in content are actually from modularization, so if any of this is out of scope with your changes, I apologize

modules/ossm-extensions-overview.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-overview.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-ref-smextension.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-ref-smextension.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-ref-smextension.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-wasmplugin-deploy.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-wasmplugin-format.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-wasmplugin-format.adoc Outdated Show resolved Hide resolved
modules/ossm-extensions-wasmplugin-format.adoc Outdated Show resolved Hide resolved
@JStickler
Copy link
Contributor Author

Added note that WebAssembly extensions not supported on IBM Power and Z systems at request of IBM technical writers.

@JStickler
Copy link
Contributor Author

Merging now to get this into main so that Eng and 3scale team can work on documenting the next steps.

@JStickler JStickler merged commit 3123487 into openshift:main Jun 23, 2022
@JStickler
Copy link
Contributor Author

/cherry-pick enterprise-4.6

@JStickler
Copy link
Contributor Author

/cherry-pick enterprise-4.7

@JStickler
Copy link
Contributor Author

/cherry-pick enterprise-4.8

@JStickler
Copy link
Contributor Author

/cherry-pick enterprise-4.9

@JStickler
Copy link
Contributor Author

/cherry-pick enterprise-4.10

@JStickler
Copy link
Contributor Author

/cherry-pick enterprise-4.11

@openshift-cherrypick-robot

@JStickler: new pull request created: #47251

In response to this:

/cherry-pick enterprise-4.6

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.

@openshift-cherrypick-robot

@JStickler: new pull request created: #47252

In response to this:

/cherry-pick enterprise-4.7

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.

@openshift-cherrypick-robot

@JStickler: new pull request created: #47253

In response to this:

/cherry-pick enterprise-4.9

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.

@openshift-cherrypick-robot

@JStickler: new pull request created: #47254

In response to this:

/cherry-pick 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.

@openshift-cherrypick-robot

@JStickler: new pull request created: #47255

In response to this:

/cherry-pick enterprise-4.11

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.

@JStickler
Copy link
Contributor Author

/cherry-pick enterprise-4.8

@openshift-cherrypick-robot

@JStickler: new pull request created: #47256

In response to this:

/cherry-pick enterprise-4.8

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.

@JStickler JStickler deleted the OSSMDOC-550 branch October 6, 2022 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants