Skip to content

Fix mii wdt filterpy #2379

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 16, 2021
Merged

Fix mii wdt filterpy #2379

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0ab46df
set default listening port if the model does not set it, otherwise is…
jshum2479 May 25, 2021
b1fd805
add comments
jshum2479 May 26, 2021
3deaeaa
set correct ssl port also
jshum2479 May 26, 2021
3a244dd
set ssl port also
jshum2479 May 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions operator/src/main/resources/scripts/model_wdt_mii_filter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,11 @@ def customizeServerIstioNetworkAccessPoint(server, listen_address):
if istio_readiness_port is None:
return
admin_server_port = server['ListenPort']
# Set the default if it is not provided to avoid nap default to 0 which fails validation.

if admin_server_port is None:
admin_server_port = 7001
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the domain is in SecuredMode?

Copy link
Member Author

Choose a reason for hiding this comment

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

istio won't work with secure mode yet

Copy link
Member

Choose a reason for hiding this comment

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

Oh! That seems like an issue. Do we have a JIRA to complete that support? I'd like to resolve that gap before the Verrazzano team hits it.

Copy link
Member

Choose a reason for hiding this comment

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

@ddsharpe @jshum2479, what would be the correct behavior for secure mode? Does this "None" check let us differentiate between when the customer has left it blank or if the admin server's default port is disabled because of secure mode?

Copy link
Member Author

Choose a reason for hiding this comment

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

Note: the filter code already have provision to setup the ssl port if the secure mode is enabled (line 423).

But, ultimately it will not work in istio environment.

  1. whenever admin port (same for secure mode) is enabled, the readiness probe /weblogic/ready is treated as management function because it started with /weblogic and it must be accessed directly read-address:adminport and not proxied it through localhost:adminport
  2. Istio always proxy it through unless there is annotation to forbid the rewrite the port traffic (essentially take it out of the mesh).
  3. operator current implementation always use the plain readiness port in the domain spec, while it can be fixed in PodStepContext but it won't fix (2).

Copy link
Member Author

Choose a reason for hiding this comment

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

As for the correct behavior, I am not sure what's the correct behavior. If secure mode is enabled, does it mean regular listen port is disabled?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that's right: https://docs.oracle.com/en/middleware/fusion-middleware/weblogic-server/12.2.1.3/lockd/secure.html#GUID-F980DB67-7CDE-4EF8-986D-D188D4EDB706

Copy link
Member Author

Choose a reason for hiding this comment

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

So the core should automatically disabled the listenPort. The question is whether the operator needs to do anything special about it. I suggest creating another issue to handle secure mode as I suspect there maybe issue in non-istio case. This PR is for missing listenPort only. For adminport/secure mode it's a dead end for istio for now.


# readiness probe
_writeIstioNAP(name='http-probe', server=server, listen_address=listen_address,
listen_port=istio_readiness_port, protocol='http', http_enabled="true")
Expand Down Expand Up @@ -413,9 +418,12 @@ def customizeServerIstioNetworkAccessPoint(server, listen_address):
model = env.getModel()
if ssl is not None and 'Enabled' in ssl and ssl['Enabled'] == 'true':
ssl_listen_port = ssl['ListenPort']
if ssl_listen_port is None:
ssl_listen_port = "7002"
elif ssl is None and isSecureModeEnabledForDomain(model['topology']):
ssl_listen_port = "7002"


if ssl_listen_port is not None:
_writeIstioNAP(name='https-secure', server=server, listen_address=listen_address,
listen_port=ssl_listen_port, protocol='https', http_enabled="true")
Expand Down Expand Up @@ -445,6 +453,9 @@ def customizeManagedIstioNetworkAccessPoint(template, listen_address):
if istio_readiness_port is None:
return
listen_port = template['ListenPort']
# Set the default if it is not provided to avoid nap default to 0 which fails validation.
if listen_port is None:
listen_port = 7001
# readiness probe
_writeIstioNAP(name='http-probe', server=template, listen_address=listen_address,
listen_port=istio_readiness_port, protocol='http', http_enabled="true")
Expand Down Expand Up @@ -473,6 +484,8 @@ def customizeManagedIstioNetworkAccessPoint(template, listen_address):
model = env.getModel()
if ssl is not None and 'Enabled' in ssl and ssl['Enabled'] == 'true':
ssl_listen_port = ssl['ListenPort']
if ssl_listen_port is None:
ssl_listen_port = "7002"
elif ssl is None and isSecureModeEnabledForDomain(model['topology']):
ssl_listen_port = "7002"

Expand Down