Skip to content

OWLS 86115: Model in Image should not use operator generated configuration overrides #2285

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

Closed
wants to merge 17 commits into from
Closed

OWLS 86115: Model in Image should not use operator generated configuration overrides #2285

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7d55190
Use WDT filtering to generate config overrides for MII
lennyphan Mar 20, 2021
1797cdc
Merge from develop branch
lennyphan Mar 23, 2021
78257a8
Fix merge mistake
lennyphan Mar 24, 2021
7438911
Add additional unit tests for situational config override environment…
lennyphan Mar 24, 2021
df14d4d
Fix for handling online dynamic update with sparse model
lennyphan Mar 26, 2021
ad00e84
Merge remote-tracking branch 'origin/develop' into OWLS-86115
lennyphan Mar 26, 2021
7ec7810
cleanup debug output
lennyphan Mar 26, 2021
eca7d2f
Merge remote-tracking branch 'origin/develop' into OWLS-86115
lennyphan Apr 7, 2021
2bcc31b
First set of changes based on review feedback
lennyphan Apr 7, 2021
b4b4345
Merge remote-tracking branch 'origin/develop' into OWLS-86115
lennyphan Apr 7, 2021
48e4252
Merge branch 'OWLS-86115' of https://github.com/oracle/weblogic-kuber…
lennyphan Apr 7, 2021
c116a66
Merge branch 'OWLS-86115' of https://github.com/oracle/weblogic-kuber…
lennyphan Apr 19, 2021
00c29e5
read domain name from topology, if available
lennyphan Apr 21, 2021
28f0e72
add missing topology.yaml file
lennyphan Apr 21, 2021
4c8ec57
try using python ast package for reading dictionary values instead of…
lennyphan Apr 22, 2021
5851ba6
minor changes based on code review
lennyphan Apr 27, 2021
2cf4b3e
Merge branch main
rjeberhard Apr 27, 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
36 changes: 29 additions & 7 deletions operator/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,15 +231,37 @@
<goal>java</goal>
</goals>
<phase>package</phase>
<configuration>
<mainClass>oracle.kubernetes.operator.helpers.CrdHelper</mainClass>
<arguments>
<argument>${project.basedir}/../kubernetes/crd/domain-crd.yaml</argument>
<argument>${project.basedir}/../kubernetes/crd/domain-v1beta1-crd.yaml</argument>
</arguments>
</configuration>
</execution>
<execution>
<id>test-python-scripts</id>
<phase>test</phase>
<goals>
<goal>exec</goal>
</goals>
<configuration>
<skip>${skip.unit.tests}</skip>
<executable>python</executable>
<workingDirectory>src/test/python</workingDirectory>
<environmentVariables>
<PYTHONPATH>${project.basedir}/src/main/resources/scripts</PYTHONPATH>
<DOMAIN_UID>sample-domain1</DOMAIN_UID>
</environmentVariables>
<arguments>
<arugument>-B</arugument>
<arugument>-m</arugument>
<argument>unittest</argument>
<argument>discover</argument>
</arguments>
</configuration>
</execution>
</executions>
<configuration>
<mainClass>oracle.kubernetes.operator.helpers.CrdHelper</mainClass>
<arguments>
<argument>${project.basedir}/../kubernetes/crd/domain-crd.yaml</argument>
<argument>${project.basedir}/../kubernetes/crd/domain-v1beta1-crd.yaml</argument>
</arguments>
</configuration>
</plugin>

</plugins>
Expand Down
4 changes: 4 additions & 0 deletions operator/src/main/java/oracle/kubernetes/operator/helpers/JobHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@ List<V1EnvVar> getConfiguredEnvVars(TuningParameters tuningParameters) {
}


if (getDomainHomeSourceType() == FromModel) {
addEnvVar(vars, ServerEnvVars.FAIL_BOOT_ON_SITUATIONAL_CONFIG_ERROR, "false");
}

String dataHome = getDataHome();
if (dataHome != null && !dataHome.isEmpty()) {
addEnvVar(vars, ServerEnvVars.DATA_HOME, dataHome);
Expand Down
4 changes: 4 additions & 0 deletions operator/src/main/java/oracle/kubernetes/operator/helpers/PodStepContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,10 @@ void addStartupEnvVars(List<V1EnvVar> vars) {
addEnvVar(vars, ServerEnvVars.AS_SERVICE_NAME, LegalNames.toServerServiceName(getDomainUid(), getAsName()));
Optional.ofNullable(getDataHome()).ifPresent(v -> addEnvVar(vars, ServerEnvVars.DATA_HOME, v));
addEnvVarIfTrue(mockWls(), vars, "MOCK_WLS");

if (getDomainHomeSourceType() == DomainSourceType.FromModel) {
addEnvVar(vars, ServerEnvVars.FAIL_BOOT_ON_SITUATIONAL_CONFIG_ERROR, "false");
Copy link
Member

@rjeberhard rjeberhard Mar 27, 2021

Choose a reason for hiding this comment

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

This is going to change our pod hash (weblogic.sha256) which means that any MII pod that is running when the operator is upgraded will be rolled. Can this instead be set in the pod start script (startServer.sh).

Copy link
Member Author

@lennyphan lennyphan Mar 28, 2021

Choose a reason for hiding this comment

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

Just to clarify the scenario you're describing for my understanding, this is when upgrading the Operator from a previous MII domain that uses sit config overrides to this version which uses the model filter? The one place that uses this environment is in startNodeManager.sh (as a server startup argument) and I couldn't determine how we know the domain source type is 'FromModel' (MII). serverStart.sh does:

if [ -f /weblogic-operator/introspector/domainzip.secure ]; then
...
fi

I think this should work. Let me make the change and verify.

Copy link
Member

@rjeberhard rjeberhard Mar 28, 2021

Choose a reason for hiding this comment

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

Right, a customer is using operator 3.1.4 and has a MII domain. The admin server and some number of managed servers are running. The customer upgrades to 3.2.0 containing your work. I would expect that pods do not roll simply because I upgrade the operator.

Thinking more deeply about this... The change to add the extra environment variable would have caused the existing pods to roll. I think that we have a workaround in that we can set this in startServer.sh. However, we likely have an earlier issue... Your change to not generate config overrides will mean that the generated domain home will be different. This change would also cause existing pods to roll, correct? If so, we should talk together with @jshum2479 and see if we can come up with a way to roll this out so that existing pods are not rolled or restarted, but newly created pods come up with the new pattern.

Copy link
Member

@jshum2479 jshum2479 Mar 29, 2021

Choose a reason for hiding this comment

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

We can add logic in start script to reset the env when the DOMAIN_SOURCE_TYPE is FromModel.

Copy link

@tbarnes-us tbarnes-us Mar 29, 2021

Choose a reason for hiding this comment

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

It doesn't look like it'd hurt to leave it alone. It'd become a NO-OP.

Copy link
Member

@jshum2479 jshum2479 Mar 29, 2021

Choose a reason for hiding this comment

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

The FAIL_BOOT_ON_SITUATIONAL_CONFIG_ERROR is for the core config override logic to failed to start the server if the override file is wrong, the start server script already have the logic to set it to true if the environment is not set, the env is for the case if user choose to override it.

I am not seeing a reason to set it for mii since we are using override and it will cause the roll because of the pod spec hash changed because the newly introduced environment variable.

Copy link

@tbarnes-us tbarnes-us Mar 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The overall point is that there's no need to change or add the old env var setting in this pull. We don't care what it's set to in the new path...

Copy link
Member Author

@lennyphan lennyphan Mar 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

in startNodeManager.sh, we set FAIL_BOOT_ON_SITUATIONAL_CONFIG_ERROR system property to 'true' by default. Managed servers fail to start/boot with the following error:

<Mar 29, 2021 8:22:23 PM GMT> <The SitConfigRequired configuration element is set to true, but no situational config files were found.>
<Mar 29, 2021 8:22:23 PM GMT> <Server failed. Reason: 141327>
<Mar 29, 2021 8:22:23 PM GMT>

The validation in WLS is in SituationalConfigManagerImpl:
if (isSitConfigRequired() && unexpiredFiles.isEmpty()) {
if (debugLogger.isDebugEnabled()) {
debugLogger.debug( "[SitConfig] isSitConfigRequired is true and no Sit Config Files exist. Failing boot.");
}
String l = ManagementLogger.logSituationalConfigRequired();
throw new ServiceFailureException(l);
}

So it looks like we should either set it to 'false' or not define it for MII.

Copy link

@tbarnes-us tbarnes-us Apr 27, 2021

Choose a reason for hiding this comment

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

The point is that we're not supporting custom or internal sit-cfg in MII regardless, so there won't be any sit-cfg that can fail, so the flag should have no effect. If removing it is safe (doesn't cause a roll), then feel free to remove it. Or feel free to leave it untouched.

}
}

private String getDomainHome() {
Expand Down
5 changes: 4 additions & 1 deletion operator/src/main/java/oracle/kubernetes/weblogic/domain/model/ServerEnvVars.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,13 @@ public class ServerEnvVars {
/** If present, pod scripts will watch for changes to override configurations and move them into place. */
public static final String DYNAMIC_CONFIG_OVERRIDE = "DYNAMIC_CONFIG_OVERRIDE";

public static final String FAIL_BOOT_ON_SITUATIONAL_CONFIG_ERROR = "FAIL_BOOT_ON_SITUATIONAL_CONFIG_ERROR";

private static final List<String> RESERVED_NAMES = Arrays.asList(
DOMAIN_UID, DOMAIN_NAME, DOMAIN_HOME, NODEMGR_HOME, SERVER_NAME, SERVICE_NAME,
ADMIN_NAME, AS_SERVICE_NAME, ADMIN_PORT, ADMIN_PORT_SECURE, ADMIN_SERVER_PORT_SECURE,
LOG_HOME, SERVER_OUT_IN_POD_LOG, DATA_HOME, ACCESS_LOG_IN_LOG_HOME, DYNAMIC_CONFIG_OVERRIDE);
LOG_HOME, SERVER_OUT_IN_POD_LOG, DATA_HOME, ACCESS_LOG_IN_LOG_HOME, DYNAMIC_CONFIG_OVERRIDE,
FAIL_BOOT_ON_SITUATIONAL_CONFIG_ERROR);

static boolean isReserved(String name) {
return RESERVED_NAMES.contains(name);
Expand Down
9 changes: 7 additions & 2 deletions operator/src/main/resources/scripts/introspectDomain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,10 @@ def addNetworkAccessPoint(self, server, nap, is_server_template):
nap_protocol = getNAPProtocol(nap, server, self.env.getDomain(), is_server_template)

if istio_enabled == 'true':
name = nap.getName()
if name.startswith('http-') or name.startswith('tcp-') or name.startswith('tls-') or name.startswith('https-'):
Copy link
Member

@rjeberhard rjeberhard Mar 27, 2021

Choose a reason for hiding this comment

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

Could this be fragile? The customer could have a nap name that starts with one of these prefixes. Do we have some earlier validation that ensures that the only names with these prefixes would be naps that were edited by our Istio functionality?

Copy link
Member Author

@lennyphan lennyphan Apr 4, 2021

Choose a reason for hiding this comment

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

I could be wrong but I couldn't find any validation for naps that have these prefixes edited by our Istio functionality. @jshum2479 is there validation for these naps that I didn't see?

Copy link
Member

@jshum2479 jshum2479 Apr 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This function is to write out the topology in the introspector output (not config override), it is trying to format the name in istio ways. If the customer already have it that way and you skipped writing to the topology yaml, then they won't be setup in the pod correctly, perhaps changing the logic to not prepend the protocol again instead of returning.

# skip istio ports already defined by WDT filtering for MII
return
http_protocol = [ 'http' ]
https_protocol = ['https','admin']
tcp_protocol = [ 't3', 'snmp', 'ldap', 'cluster-broadcast', 'iiop', 'sip']
Expand Down Expand Up @@ -1606,10 +1610,11 @@ def introspect(self):
tg = TopologyGenerator(self.env)

if tg.validate():
SitConfigGenerator(self.env).generate()
DOMAIN_SOURCE_TYPE = self.env.getEnvOrDef("DOMAIN_SOURCE_TYPE", None)
if DOMAIN_SOURCE_TYPE != "FromModel":
SitConfigGenerator(self.env).generate()
BootPropertiesGenerator(self.env).generate()
UserConfigAndKeyGenerator(self.env).generate()
DOMAIN_SOURCE_TYPE = self.env.getEnvOrDef("DOMAIN_SOURCE_TYPE", None)

if DOMAIN_SOURCE_TYPE == "FromModel":
trace("cfgmap write primordial_domain")
Expand Down
4 changes: 4 additions & 0 deletions operator/src/main/resources/scripts/model-filters.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,9 @@
"discover": [
],
"update": [
{ "name": "update_mii_filter", "path": "/u01/wdt/weblogic-deploy/lib/wdt_mii_filter.py" }
],
"validate": [
{ "name": "validate_mii_filter", "path": "/u01/wdt/weblogic-deploy/lib/wdt_mii_filter.py" }
]
}
19 changes: 18 additions & 1 deletion operator/src/main/resources/scripts/modelInImage.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ WDT_OUTPUT="/tmp/wdt_output.log"
WDT_BINDIR="${WDT_ROOT}/bin"
WDT_FILTER_JSON="/weblogic-operator/scripts/model-filters.json"
WDT_CREATE_FILTER="/weblogic-operator/scripts/model-wdt-create-filter.py"
WDT_MII_FILTER="/weblogic-operator/scripts/wdt_mii_filter.py"
UPDATE_RCUPWD_FLAG=""
WLSDEPLOY_PROPERTIES="${WLSDEPLOY_PROPERTIES} -Djava.security.egd=file:/dev/./urandom"
ARCHIVE_ZIP_CHANGED=0
Expand Down Expand Up @@ -319,9 +320,9 @@ function createWLDomain() {
fi

# copy the filter related files to the wdt lib

cp ${WDT_FILTER_JSON} ${WDT_ROOT}/lib/model_filters.json
cp ${WDT_CREATE_FILTER} ${WDT_ROOT}/lib
cp ${WDT_MII_FILTER} ${WDT_ROOT}/lib
Copy link

@tbarnes-us tbarnes-us Apr 27, 2021

Choose a reason for hiding this comment

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

consider appending || trace SEVERE "cp failed" && exitOrLoop to each of the cp commands (fail-fast)


# check to see if any model including changed (or first model in image deploy)
# if yes. then run create domain again
Expand Down Expand Up @@ -793,6 +794,13 @@ function generateMergedModel() {

export __WLSDEPLOY_STORE_MODEL__="${NEW_MERGED_MODEL}"

local wdtArgs=""
wdtArgs+=" -oracle_home ${ORACLE_HOME}"
wdtArgs+=" ${model_list} ${archive_list} ${variable_list}"
wdtArgs+=" -domain_type ${WDT_DOMAIN_TYPE}"

trace "About to call '${WDT_BINDIR}/validateModel.sh ${wdtArgs}'."

${WDT_BINDIR}/validateModel.sh -oracle_home ${ORACLE_HOME} ${model_list} \
${archive_list} ${variable_list} -domain_type ${WDT_DOMAIN_TYPE} > ${WDT_OUTPUT} 2>&1
ret=$?
Expand Down Expand Up @@ -917,6 +925,15 @@ function wdtUpdateModelDomain() {
# make sure wdt create write out the merged model to a file in the root of the domain
export __WLSDEPLOY_STORE_MODEL__=1

local wdtArgs=""
wdtArgs+=" -oracle_home ${ORACLE_HOME}"
wdtArgs+=" -domain_home ${DOMAIN_HOME}"
wdtArgs+=" ${model_list} ${archive_list} ${variable_list}"
wdtArgs+=" -domain_type ${WDT_DOMAIN_TYPE}"
wdtArgs+=" ${UPDATE_RCUPWD_FLAG}"

trace "About to call '${WDT_BINDIR}/updateDomain.sh ${wdtArgs}'."

${WDT_BINDIR}/updateDomain.sh -oracle_home ${ORACLE_HOME} -domain_home ${DOMAIN_HOME} $model_list \
${archive_list} ${variable_list} -domain_type ${WDT_DOMAIN_TYPE} ${UPDATE_RCUPWD_FLAG} > ${WDT_OUTPUT} 2>&1
ret=$?
Expand Down
Loading