Skip to content

Implementation for podman to be used for container images #1357

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 7 commits into from
Nov 22, 2023
Merged

Implementation for podman to be used for container images #1357

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
60baa9e
add setenv script
Jdubrick Nov 20, 2023
1e61932
update run script to include podman capabilities
Jdubrick Nov 20, 2023
3870586
update documentation to reflect podman implementation
Jdubrick Nov 20, 2023
cce5fe4
update documentation to remove duplicate entries
Jdubrick Nov 21, 2023
965bdcd
Merge branch 'devfile:main' into podman-implementation
Jdubrick Nov 22, 2023
f2d6c95
add inline setting of alias to remove need for additional script
Jdubrick Nov 22, 2023
d086125
fix unset variable
Jdubrick Nov 22, 2023
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: 11 additions & 2 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,22 @@ Before contributing to this repository for the first time, please review our pro

### Building

To build the CRD and the various schemas, you don't need to install any pre-requisite apart from `docker`.
In the root directory, just run the following command:
To build the CRD and the various schemas, you don't need to install any pre-requisite apart from `docker` or `podman`.
In the root directory, if you are using `podman` first run `export USE_PODMAN=true`. Then for either `docker` or `podman` run the following command:

```console
bash ./docker-run.sh ./build.sh
```

### Typescript model

Typescript model is generated based on JSON Schema with help of <https://github.com/kubernetes-client/gen>.
To generate them locally run:

```console
bash ./build/typescript-model/generate.sh
```

### Testing

Find more information about tests in the [testing document](test/README.md).
Expand Down
18 changes: 3 additions & 15 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Sources for this API are defined in Go code, starting from the
[devworkspace_types.go source file](pkg/apis/workspaces/v1alpha2/devworkspace_types.go)

From these Go sources, several files are generated:

- A Kubernetes Custom Resource Definition(CRD) with an embedded OpenApi schema,
- json schemas (in the [schemas](schemas) folder) generated from the above CRD, to specify the syntax of:
- the DevWorkspace CRD itself;
Expand All @@ -34,21 +35,7 @@ Release details and process are found in [Devfile Release](RELEASE.md)

## How to build
Copy link
Contributor

@yangcao77 yangcao77 Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

this section is duplicating the content in contributing doc. We shouldn't spent effort to maintain same content in separate docs. Please use the link to contributing.md, e.g. ./CONTRIBUTING.md#building

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree about the duplication :) Thinking that in general the contribution guidelines are more related to information about how someone could open a PR or an issue, I was wondering if the contributing.md is the right file to maintain this section? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing that out! To remove the duplication I agree with contributing.md probably not being the right spot for the build instructions, I'll remove it from contributing and keep in in the readme.md only

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yangcao77 @thepetk please see the latest commit for my updated changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

should directly link to the building section. not the top of the contributing doc


In order to build the CRD and the various schemas, you don't need to install any pre-requisite apart from `docker`.
In the root directory, just run the following command:

```console
bash ./docker-run.sh ./build.sh
```

### Typescript model

Typescript model is generated based on JSON Schema with help of <https://github.com/kubernetes-client/gen>.
To generate them locally run:

```console
bash ./build/typescript-model/generate.sh
```
For information about building this project visit [CONTRIBUTING.md](./CONTRIBUTING.md).

## Specification status

Expand All @@ -61,6 +48,7 @@ In order to test existing or new Devfile 2.0 or DevWorkspace sample files in a s
[![Contribute](https://img.shields.io/badge/developer-workspace-525C86?logo=eclipse-che&labelColor=FDB940)](https://workspaces.openshift.com/f?url=https://github.com/devfile/api)

As soon as the devworkspace is opened, you should be able to:

- open the `yaml` files in the following folders:
- `samples/`
- `devfile-support/samples`
Expand Down
6 changes: 6 additions & 0 deletions docker-run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# Allow setting of podman environment var in the script runtime
shopt -s expand_aliases
set -eux

# git ROOT directory used to mount filesystem
GIT_ROOT_DIRECTORY=$(git rev-parse --show-toplevel)
Expand All @@ -23,6 +26,9 @@ WORKDIR="/projects/src/${GO_MODULE}"
# Container image
IMAGE_NAME="quay.io/devfile/kubernetes-api-build-prerequisites:latest"

# Run script to set env var for podman if necessary
. ./setenv.sh

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I like hiding the alias in a separate script and controlled by an env var -- it's hiding a lot of functionality here (if you don't notice these two lines at the top it looks like it just uses docker).

init() {
BLUE='\033[1;34m'
GREEN='\033[0;32m'
Expand Down
25 changes: 25 additions & 0 deletions setenv.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/bin/bash

#
# Copyright Red Hat
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# This script aliases the docker cli if the environment variable USE_PODMAN is set to true.

# default value is false if USE_PODMAN is unset or null
podman=${USE_PODMAN:-false}
if [ ${podman} == true ]; then
alias docker=podman
echo "setting alias docker=podman"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not inline this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I do have the same question, why using a separate script just for setting alias?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reasoning for having it in a separate script was only because I was going off of prior work done here by someone else. That repo utilizes the same setenv.sh script being used alongside an env variable to toggle between podman or docker.

If it's preferred to be inline that change can definitely be made.

Copy link
Contributor

@amisevsk amisevsk Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The way I've usually done it in other repos is to just use a variable, e.g.

DOCKER=docker
if [ <use podman condition> ]; then
  DOCKER=podman
fi

$DOCKER build -t <image> -f <file> <context>

this also avoids the need for setting shopt -s expand_alias.

Copy link
Contributor Author

@Jdubrick Jdubrick Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In my last commit I added the setting of the alias inline so we don't need another script to be doing that work. I initially tried to go with your way @amisevsk with the use of a variable but found that when it was trying to call the ./build.sh script it was failing with a seg fault, this was not happening with setting the alias. I'm not sure why so more investigation will have to be done into that if we want to not use an alias and rather a variable.