Split MII zips across Config Maps #2095
Conversation
russgold
requested review from
rjeberhard,
tbarnes-us,
doxiao,
lennyphan and
ankedia
|
High level initial questions/comments: (1) Could we have 3 fixed named config maps instead of a variable number of configmaps? One for each of the two big MII zips, and a third 'main' one for everything else? Putting the contents in a predictable named location would make theend-user-experience, end-user-debugging, documentation, predictability, testability, etc easier. I realize this is a trade-off versus generic handling of the data, but I think it's a a worthy one. For example, one would know that the "MII primordial' is always in the 'primordial' configmap, and the domain key is always in the 'main' configmap, etc. It would also conveniently remove the big MII zips that clutter the 'main' configmap which also contains the shell scripts, security keys, etc, making it easier to examine the contents of that particular configmap. Finally, it'd make the MII path a more intuitive superset of the behavior of the other domain home source types. (2) Is there are JIRA to track integration testing? (3) Is the upgrade of a running domain handled? (4) There are no documentation changes in this pull. I think the naming pattern and existence of the introspector configmap is documented in at least a couple of places. |
No, because we don't know how large the zips might be. Johnny suggested that in the JIRA, and Ryan gave this explanation.
Not to my knowledge
The code path is unchanged. If the old logic to restore the MII zips was called before, it still will be.
If you can point me to them, I can update them. |
I don't think that argument precludes the essence of the suggestion. For example, the primordial zip could go to a dedicated set of one or more configmaps. IMO the ease-of-use and 'debuggability' gain would be worth this refinement.
Please file one. It should be a relatively high priority.
So the original configmap name is unchanged? Can you please provide an example of how the upgrade would work? For example, would the managed servers need to be rolled to load new configmaps that they had'nt mounted before?
(5) Johnny has an external utility that he uses for debugging that pulls the domain home out of the configmaps, decrypts them, and expands them. I don't know if this pull covers that? If not, ping Johnny for its location in github. |
Can you explain how such a change improves ease-of-use and 'debuggability' ? This should all be internal to the operator.
If a change required a different number of config maps, that also requires the pods to have more volumes and mounts, which would cause them to roll. That's already in place. In addition, the checksum of the domain zip is already consulted, and would change, even if the number of maps does not.
I've updated a couple that seem relevant.
If it's external, then no. Having an external tool like that cares about internal representations sounds potentially problematic. It might be worthwhile to include something like that as part of the operator scripts, using the functions I've extracted to the MII script. @jshum2479 |
|
How about naming as primoridal-1, primordial-2 ... and then domain-config-1, domain-config-2, main-configmap where everything else go? The primordial is the vanilla domain without any configuration, when we start to support other domain source type or existing domain template grew substantially in size, it is possible to exceed 1m. The domain config today can exceeds 1m , say for example one user create 250 servers, 10000 JMS modules and even gzip the config/**/*.xml It will be nice if we can name it with some recognizable way but the process should be transparent for the users. Is there a way to manually reconstruct the zip from various configmaps in case we have to debug it? Also, are we just splitting the zips or other non-zip files can be placed in any one of the configmap? We have a utility to get the merged model from the current config map, do we have to change it? How are you testing the splitting and reconstruction now? |
|
The tool for decryption the merged model is in .//src/integration-tests/bash/show_merged_model.sh |
|
The code is unit tested; there is not currently an integration test that takes advantage of this feature. The code works as implemented; what is the tremendous value to be gained by putting different files in different config maps? As long as no zips are large enough to need it, this functionality will not be evident to anyone - everything goes into the same config maps as before. The script modelInImage.sh has functions 'restoreDomainConfig' and 'restorePrimordialDomain' which will handle all of the reconstruction of the domains from the config maps. I presume that your tool could simply call those. In theory, any type of file could be split, but there is no script support at present for reassembling them. |
|
That's good, today in practice zips too big only happens if user has lots of config/**/*.xml |
It all depends on how much usability and debuggability is valued. Expanding on earlier: (1) It's nice to be able to guarantee that the same file is in the same configmap in all use cases. This makes them easy to find. |
| cat ${OPERATOR_ROOT}/introspector*/${1} > /tmp/domain.secure | ||
| base64 -d "/tmp/domain.secure" > /tmp/domain.tar.gz || return 1 | ||
|
|
||
| tar -xzf /tmp/domain.tar.gz || return 1 |
There was a problem hiding this comment.
@jshum2479 This is unrelated to Russ' enhancement, but shouldn't the old /tmp file be removed upon success? I assume it takes up a good amount of space.
tar -xzf /tmp/domain.tar.gz || return 1
rm /tmp/domain.tar.gz
There was a problem hiding this comment.
modelInImage only affects the introspect pod which will go away anyway, but since this method will be called in startServer.sh, we should clean up the specific file.
| # $1 the name of the encoded file in the config map | ||
| function restoreEncodedTar() { | ||
| cd / || return 1 | ||
| cat ${OPERATOR_ROOT}/introspector*/${1} > /tmp/domain.secure |
There was a problem hiding this comment.
Append || return 1 to the cat... (not strictly needed as the next line will fail if the cat fails)
With the exception of the zip files if they are large enough, the other entries are exactly where they have always been
If the tool that examines the map is not interested in them, it can simply ignore them. Everything else should work just as it always has. |
|
At this point, Jenkins runs seem to be working other than intermittent failures to access K8S, which are happening for multiple branches. |
Under some conditions, the zips generated by an MII domain may be too large to fit in a single config map, which has a total limit of 1 MB. To address that, this PR includes code to split the encoded archives across multiple config maps as needed, and reassemble them as part of starting up the server.
As this requires changes to the shell scripts, this PR also includes a new Maven mojo to run bash unit tests, using shunit2.
Note: the mojo is not unique to this project and contains no Oracle-proprietary code. Since other projects may wish to unit tests scripts, we should look into releasing the plugin separately in the future.