Skip to content
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

Introduce zos as platform #1095

Merged
merged 1 commit into from Aug 7, 2021
Merged

Introduce zos as platform #1095

merged 1 commit into from Aug 7, 2021

Conversation

najohnsn
Copy link
Contributor

As discussed in the 1/27/2021 OCI developer meeting, these changes introduce IBM z/OS as a Native Platform for OCI. Changes to code, JSON schema, and markdown documentation is included.

Other than zos being added a platform and having its own platform-specific configuration, nothing new is really added with this. The expectation is that additional pull requests (preceded by development discussions, if needed) will be used to request any z/OS-unique configuration properties as the IBM initiative to support OCI containers on z/OS progresses.

Note on z/OS devices: it is the only property in the z/OS-specific configuration object. This was copied from how devices can be specified for Linux. Specifying devices like this would be a perfectly valid way to define/create devices in a container on z/OS. However, its usefulness on z/OS is questionable. It is being included in this PR so that something is defined in the z/OS-specific configuration object. It very well may turn out that the ability to specify devices to be created in a z/OS container may be unnecessary and that the definition should be replaced with more useful properties.

Signed-off-by: Neil Johnson najohnsn@us.ibm.com
Contributors: @SteeleDesmond and @najohnsn

@tianon
Copy link
Member

tianon commented Mar 12, 2021

It very well may turn out that the ability to specify devices to be created in a z/OS container may be unnecessary and that the definition should be replaced with more useful properties.

Given that this is speculative, and anything we merge might end up in a release, perhaps this should wait until there's something concrete to include? Alternatively/also, it might be prudent to mark all the bits related to this platform as "experimental" or "in-progress" in some way until it's deemed at least minimally feature complete (if the goal is to merge adjustments incrementally before the full functionality is known/working).

@najohnsn
Copy link
Contributor Author

najohnsn commented Apr 6, 2021

Sorry for the delay. Yes, just as you said, the goal is the merge adjustments incrementally.

Let me explain a bit more about /dev on z/OS. Although the major/minor number of devices are different on z/OS when compared to Linux, they can be created in the same way - mknod. The more interesting thing on z/OS is that the number of different kinds of devices is fairly limited and that they are created by the system either during initialization or on demand. For example, if a request to open /dev/console is done and the file is not there, the system will create a /dev/console with an appropriate major/minor number. It is this on-demand creation that led me to the statement that it is not very useful on z/OS.

Do you think that just an in progress note on the zos line in the list of Platforms in spec.md would satisfy the raised concern?

@najohnsn
Copy link
Contributor Author

najohnsn commented Apr 9, 2021

I added a commit to Indicate work in progress.
It's not the changes I was leaning toward in my previous comment, but I think it covers more:
Added "This document is a work in progress." to config-zos.md.

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

That's good enough for me! 😅 Thanks 👍

LGTM

(Does this mean we're going to get some sweet Z systems to play/CI with? :trollface:)

@najohnsn
Copy link
Contributor Author

IBM is looking at providing z/OS systems for CI. 🙂

It's been a while. Is anything else needed for this PR to get approved/merged?

@tianon
Copy link
Member

tianon commented Jun 25, 2021

I'd suggest a rebase (to try and fix the CI failure) and a squash (since I don't think we want three commits that together amount to "this is coming"). 😅

(Maybe that will help encourage other members of @opencontainers/runtime-spec-maintainers to opine/review.)

Comment on lines 30 to 31
// Zos is platform-specific configuration for z/OS based containers.
Zos *Zos `json:"zos,omitempty" platform:"zos"`
Copy link
Member

Choose a reason for hiding this comment

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

In Go, initialisms generally remain capitalized. Seems like the field (and type) should be ZOS instead of Zos.

Suggested change
// Zos is platform-specific configuration for z/OS based containers.
Zos *Zos `json:"zos,omitempty" platform:"zos"`
// ZOS is platform-specific configuration for z/OS based containers.
ZOS *ZOS `json:"zos,omitempty" platform:"zos"`

Signed-off-by: Neil Johnson <najohnsn@us.ibm.com>
Signed-off-by: Steele Ray Desmond <steele.desmond@ibm.com>
@najohnsn
Copy link
Contributor Author

najohnsn commented Jul 9, 2021

@tianon @samuelkarp Thanks for the suggestions! The 3 commits have been replaced by a single commit that incorporates your suggestions.

Copy link
Member

@samuelkarp samuelkarp left a comment

Choose a reason for hiding this comment

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

LGTM, though I'm not a maintainer of the spec.

@vbatts vbatts merged commit 82ab996 into opencontainers:master Aug 7, 2021
@AkihiroSuda AkihiroSuda mentioned this pull request Jan 24, 2023
@AkihiroSuda AkihiroSuda added this to the v1.1.0 milestone Feb 1, 2023
@AkihiroSuda AkihiroSuda mentioned this pull request Jun 26, 2023
12 tasks
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.

None yet

5 participants