-
Notifications
You must be signed in to change notification settings - Fork 1.8k
TP12 Modularization Work #15681
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
TP12 Modularization Work #15681
Conversation
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 skimmed the modules, assembly files, and rendered assemblies for modularization issues but didn't do a proper peer review. I think it's looking good! I have some suggestions and requests. Please let me know if you have questions.
service_mesh/service_mesh_install/prepare-to-deploy-applications-ossm.adoc
Outdated
Show resolved
Hide resolved
24ea47d
to
883dda9
Compare
@geekspertise super late, but went through the comments and docs yesterday and today and it LGTM |
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 see you are actively working on this. I'll wait you to push your changes to resume my review.
cdbabd2
to
eefc691
Compare
I've added the 3scale content for TP12 so @dfennessy and the 3scale team can start their 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.
A few comments, but other than that, it looks good to me.
service_mesh/service_mesh_install/prepare-to-deploy-applications-ossm.adoc
Outdated
Show resolved
Hide resolved
4cb036a
to
d16205e
Compare
@jwendell You can resume your review. I pushed a lot of changes last night and some additional ones this morning based on comments from @brian-avery . |
d16205e
to
6969f47
Compare
08e6bc3
to
66216e1
Compare
8b18caa
to
7d7277d
Compare
7d7277d
to
10ee540
Compare
I don't think we want this. First of all, it requires the operator to be given superpowers (i.e. the permission to create cluster roles/bindings) - this is usually reserved for cluster admin users only. The support for this was mainly for backward compatibility with upstream Istio users and how they've been using Kiali since the beginning. Secondly, this is here to support "cluster-wide mesh" - but Maistra is going to remove their support for non-MT (cluster) meshes - so we should not be requiring people to give the operator these superpowers because it will never need them when Maistra removes non-MT mode. So, in short, we need to take that out once Maistra removes their support for non-MT mode (which, IIUC, is next TP release). They are going to remove that support for GA is what I was told. |
The preview will be available shortly at: |
c25b0cb
to
b96fa64
Compare
@kalexand-rh and @vikram-redhat This PR is ready to merge. I rebased and commented out my changes to the topic yaml. The TP12 containers are live and @knrc says we are good to merge. I also touched base with @JStickler to ensure we are all on the same page. |
b96fa64
to
4095386
Compare
/cherrypick enterprise-4.1 |
@kalexand-rh: new pull request created: #15996 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. |
CP to 4.2 via #16030 |
This PR is a WIP that currently includes:
Additional work necessary before merge:
Can @knrc @tvieira and @brian-avery start reviewing the docs to ensure post-mod accuracy?
Can @kalexand-rh or @vikram-redhat review for modularization issues?
Can @JStickler review to ensure we are ready to form Voltron when TP12 is complete?