Skip to content

Conversation

@OldNavi
Copy link

@OldNavi OldNavi commented Aug 14, 2018

Hello all,

i've seen many attempts to enable durable task plugin on native z/OS unix system services, i had it running fine almost for one year - merging base master branch with my changes locally at my site and now decided to offer it. Could you please have a look if this enablement changes can be merged in the plugin codebase or advise what can i amend/change ?

@OldNavi
Copy link
Author

OldNavi commented Aug 22, 2018

Hi @jglick - is there any chance that this PR can be reviewed and accepted ?

@RamchandV
Copy link

@jglick waiting for this change , please review, @OldNavi thankyou for helping the community.

@jglick
Copy link
Member

jglick commented Aug 23, 2018

Is this intended to supersede #28 and/or #76?

jglick
jglick previously requested changes Aug 23, 2018
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

IIUC this is about handling a system in which the native encoding is not a superset of ASCII, which as noted in jenkinsci/jep#173 is explicitly not supported for now. The issue is not about process output, which you could already explicitly request to be transcoded from the EBCDIC in jenkins-log.txt (or output.txt), but about the encoding of script.sh and jenkins-result.txt. Should these files always be written/read in system default encoding? That is probably the right move, but it is unsafe to do so without some sort of automated test—minimally with a commonplace non-UTF-8 encoding and demonstrating that non-ASCII content in script.sh is correctly interpreted; ideally with a non-ASCII-superset encoding and demonstrating that jenkins-result.txt is correctly interpreted, though I am not sure it is even possible to run a Linux Docker container in such an encoding and have it be honored by /bin/sh.

CountingOutputStream cos = new CountingOutputStream(transcodedSink);
try {
log.act(new WriteLog(lastLocation, new RemoteOutputStream(cos)));
sink.flush();
Copy link
Member

Choose a reason for hiding this comment

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

What does this have to do with z/OS support?

Copy link
Author

Choose a reason for hiding this comment

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

Well, this is due to weird IBM Java behavior on z/OS. Without flushing stream some of the job output isn't timely appears in a log. No side effects on OpenJDK or Sun Java.

Copy link
Member

Choose a reason for hiding this comment

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

Without this, does the output not appear at all, or does it just take a long time?

@jglick
Copy link
Member

jglick commented Aug 23, 2018

To be clear, I did already try to test UTF-16 as an encoding on a particular sh step using an existing test container, which predictably resulted in complete junk. I did not yet try to create a new test container natively set to UTF-16 and use it to just run regular stuff with ASCII output. Since no Pipeline devs have access to a z/OS environment to test anything interactively, much less in CI, that is the closest we could come.

@OldNavi
Copy link
Author

OldNavi commented Aug 24, 2018

A couple of notices why i had to encode script sh to EBCDIC on the z/OS USS platform. First thing - we couldn't rely on a system default encoding in z/OS Java because it doesn't reflect actual system encoding at all (by default z/OS IBM Java uses UTF8 BTW). Setting it with -Dfile.encoding=EBCDIC code page can avoid transcoding of script.sh but it is global setting and affects a lot of other things in a jenkins agent and plugins behavior. Technically there is a way to have a script.sh in ASCII charset on z/OS USS and execute it but this requires to call a z/OS chtag utility to tag script.sh as file required for transcoding ( https://www.ibm.com/support/knowledgecenter/SSLTBW_2.2.0/com.ibm.zos.v2r2.bpxa500/chtag.htm )
which again means that we need to create some script and execute beforehand. I'm using jenkins to build z/OS legacy code like PL/I and Assembler and another dark side of the moon is an output of programs running on MVS side of z/OS which can be called via script.sh but doesn't accept ASCII encoding in any way.
So in general i've tried to isolate z/OS USS behavior from other platforms.
i've also adopted git-client-plugin to work on z/OS USS - same issues are there.

@OldNavi
Copy link
Author

OldNavi commented Aug 24, 2018

Some comments why setting encoding at sh DSL is not helping on z/OS and i have to force encoding inside plugin (1-st reason is because writing script is only UTF-8 )

So running simple pipeline on z/OS with sh step set to IBM1047 encoding
2018-08-24 19 19 45

We are getting the following output
2018-08-24 19 18 52
It's clear that jenkins-result.txt can't be parsed because in STATUS_CHECK_INSTANCE invocation we don't do transcoding. in pull request i've changed it to honor charset from FileMonitoringTask

2018-08-24 19 19 12

i personally don't like a check i've wrote here - maybe it is better to change a behavior and honor sh encoding setting when writing a script.sh - but it is a bold move, as we aren't going to transcode an output only but an input too
`
String encoding = os == OsType.ZOS ? "IBM1047" : null;
if(encoding != null) charset(Charset.forName(encoding));
ShellController c = new ShellController(ws);

.....

shf.write(s, encoding==null ? "UTF-8" : encoding);
shf.chmod(0755);`

@OldNavi
Copy link
Author

OldNavi commented Aug 24, 2018

For this PR to work it needs JEP-206 implemented so there is a prerequisite of Pipeline Nodes and Processes Plugin > 2.20

@jglick
Copy link
Member

jglick commented Aug 25, 2018

BTW it is pointless to work in this area until after #60 is merged.

@jglick jglick dismissed their stale review August 25, 2018 16:31

stale

@jglick jglick added the on-hold label Aug 25, 2018
@jglick jglick self-requested a review August 25, 2018 16:31
@OldNavi
Copy link
Author

OldNavi commented Sep 4, 2018

Hi Jesse,

Do you know any timeline when #60 is going to be merged, so i'll be able to continue to adopt my changes on top of it? We have just finished adopting git cli client to work on z/OS USS and merged into master jenkinsci/git-client-plugin#362 so half work of z/OS USS enablement for Jenkins is done. :-)

@RamchandV
Copy link

Hi Jesse,
Do you know any timeline when #60 is going to be merged, so i'll be able to continue to adopt my changes on top of it? We have just finished adopting git cli client to work on z/OS USS and merged into master jenkinsci/git-client-plugin#362 so half work of z/OS USS enablement for Jenkins is done. :-)

#60 is merged now

@OldNavi
Copy link
Author

OldNavi commented Sep 13, 2018

Work in progress. Not yet ready for merge.

@OldNavi
Copy link
Author

OldNavi commented Nov 15, 2018

@jglick - I refactored code just to be specific on z/OS platform only, trying to avoid changes which could potentially affect open platforms. Can you have a look if it is okay now ?

@jglick jglick self-requested a review November 16, 2018 20:17
@gorbadoc
Copy link

I, too, am waiting for this. Any chance it can get merged in the next month or so?

I am the front of a wave that will (hopefully) push a bunch of CI into Jenkins if I can show developers that all they need to do is commit something and their development branch will get built across a bunch of platforms. This is already done on a limited basis for UNIX variants but is only set up for production builds.

What can I do to help expedite this? I used to be a contributor to Eclipse CDT and was a committer on coreboot.

@OldNavi
Copy link
Author

OldNavi commented Jan 15, 2019

Hi Devin,

Is there any chance to have a progress on this PR ?

@dwnusbaum
Copy link
Member

@OldNavi Sorry that I have not had a chance to look yet. I will try to review the PR in the next few weeks. Part of the problem is that I am simply not familiar with z/OS, so it is difficult for me to review the changes, and without any tests I am concerned that we will regress the new functionality. When I have time to review I want to try to create a test using a Docker fixture for a container with a non-ASCII superset encoding (similar to tests like this one) so that we can have automated tests for at least some of the behavior.

@gorbadoc
Copy link

gorbadoc commented Jan 15, 2019 via email

@OldNavi
Copy link
Author

OldNavi commented Jan 15, 2019

and without any tests I am concerned that we will regress the new functionality. When I have time to review I want to try to create a test using a Docker fixture for a container with a non-ASCII superset encoding...

I've tried to wrap z/OS logic just not to interfere with open systems at all. As for testing, I was thinking about "emulating" z/OS behavior with Docker - but it seems to be not trivial as for open systems. Playing with System.setProperty("file.encoding", "IBM1047") in runtime has no effect, as it should be only set up at startup time with -Dfile.encoding... but setting that at start up time, completely breaks Jenkins agent behavior...
In general z/OS Java setup in z/OS is kind of mixed behavior in a difference for open systems... JVM itself runs with ASCII charset specified either UTF-8 or ISO8859-1 (which bidi maps to IBM1047). Shell scripts need to be in EBCDIC code pages (IBM1047 by default) so we do a transcoding... Shell return code and output also comes in EBCDIC charset.

@hogstrom
Copy link

@dwnusbaum Just a data point. I have a z/OS system on which I installed this fix and it correctly addressed the problem and I was able to build. I'm aware of a large number of z/OS customers that are incorporating Jenkins to improve their CI/CD pipeline and this is a needed fix for them.

I understand that you do not have access to a z/OS system. I offer to get you access to one and to take you through the system to improve your familiarity of the problem and how the platform works. I'm working on Project Zowe and we need this functionality to build open source software on Z as well. The system is on the Internet and we can spend whatever time is needed via WebEx if that would be a benefit to you. Contact information is in my Git Profile.

Thanks for your consideration on bringing this issues to a close.

@OldNavi Thanks for the work here ... very much appreciated.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

I do not pretend to follow what this is doing, but with one smallish change, it can be made to not affect non-z/OS platforms at all, and I trust that z/OS users can figure out how to set up some CI system to check for regressions in public releases after the fact.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Thank you, all z/OS specifics finally seem to be fenced off cleanly!

Copy link
Member

@dwnusbaum dwnusbaum left a comment

Choose a reason for hiding this comment

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

Now that the z/OS behavior has been separated cleanly from other platforms in the recent changes I am happy to merge the PR since there is nothing that we could test here anyway.

Thanks everyone for the comments and for your interest in Jenkins Pipeline. To clarify my earlier comment, my point was not really about the correctness of this change (I trust that it works for you all), but that without automated testing I can make no guarantee that further changes to this plugin will not break the behavior on z/OS, since I simply do not have any time to dedicate to manually testing specific platforms. As Jesse said, I would highly recommend creating some kind of nightly test run to check for regressions on a z/OS system or similar if you are relying on this functionality.

Thanks for the PR and your patience @OldNavi!

@hogstrom
Copy link

@dwnusbaum Devin, as part of the Zowe project that is building z/OS Open Source we are creating instances to be used by open source projects for build testing. Although a small portion of the community, if we could help to provide and maintain an instance for the Jenkins community we'd be interested in helping out. Please let me know if there is interest in including this platform as part of the Jenkins testing.

@dwnusbaum
Copy link
Member

@hogstrom I appreciate your offer, and perhaps someone else on this PR would be interested in looking into it, but I am not able to commit to driving that effort myself.

@dwnusbaum dwnusbaum merged commit 812a8e6 into jenkinsci:master Jan 22, 2019
@dwnusbaum dwnusbaum changed the title z/OS Unix System Services enablement [JENKINS-37341] z/OS Unix System Services enablement Jan 31, 2019
@ghost
Copy link

ghost commented Aug 27, 2019

I just ran into this issue and found that the fix does NOT work on intel Jenkins servers. Just Unix based systems....

@OldNavi
Copy link
Author

OldNavi commented Aug 27, 2019

I just ran into this issue and found that the fix does NOT work on intel Jenkins servers. Just Unix based systems....

@nlopez59 - can you share more details ?

@gin-nader
Copy link

I'm hitting an issue where the jenkins-result.txt file is getting tagged as ASCII on a z/OS machine. This is causing the StatusCheckWithEncoding(getCharset()) check at

status = statusFile.act(new StatusCheckWithEncoding(getCharset()));
to throw an Exception.

Should I open a PR for the fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants