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

Added podman scripts #128

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Victoremepunto
Copy link
Contributor

This is still a WIP

Provides Podman based bash scripts and pod.yaml template to deploy locally the Ingress service stack using podman.

Copy link

@akofink akofink left a comment

Choose a reason for hiding this comment

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

Thanks for the initiative, @Victoremepunto! A couple comments so far inline.


PODMAN_NETWORK="cni-podman1"
PODMAN_GATEWAY=$(podman network inspect $PODMAN_NETWORK | jq -r '..| .gateway? // empty')

Copy link

Choose a reason for hiding this comment

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

Suggested change
$RUNNER network inspect $PODMAN_NETWORK &>/dev/null || $RUNNER network create $PODMAN_NETWORK

#!/usr/bin/env bash

set -e

Copy link

Choose a reason for hiding this comment

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

I think it'd be useful (to me, at least) to have a $RUNNER env variable, defaulted to podman:

Suggested change
RUNNER=${RUNNER:podman}

Then, for example, you can invoke the script like this:

RUNNER='sudo podman' ./scripts/podman-bootstrap.sh

How have you been running it? With sudo ./scripts/podman-bootstrap.sh ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeh exactly, as this version needs to manipulate networks and at the moment podman requires sudo privileges for it. not sure I want to keep it that way, but one of the things we will be dropping if going for rootless podman containers is dns domain name resolution. the alternative in rootless podman would be to keep all services inside the same pod, and listen to localhost:{service-port} when communicating with other containers.

@subpop
Copy link
Contributor

subpop commented Jul 22, 2020

Take a look at the shellcheck output on these scripts. There are a few suggestions worth fixing.

[link@localhost insights-ingress-go]$ shellcheck --format=gcc scripts/*.sh
scripts/check-response-code.sh:10:35: warning: This { is literal. Check expression (missing ;/\n?) or quote it. [SC1083]
scripts/check-response-code.sh:10:45: warning: This } is literal. Check expression (missing ;/\n?) or quote it. [SC1083]
scripts/check-response-code.sh:10:63: note: Double quote to prevent globbing and word splitting. [SC2086]
scripts/check-response-code.sh:12:6: note: Double quote to prevent globbing and word splitting. [SC2086]
scripts/check-response-code.sh:12:32: note: Double quote to prevent globbing and word splitting. [SC2086]
scripts/insights-pod.sh:6:1: warning: WORKDIR appears unused. Verify use (or export if used externally). [SC2034]
scripts/wait-for-minio.sh:7:5: warning: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate. [SC2124]
scripts/wait-for-minio.sh:16:6: note: Double quote to prevent globbing and word splitting. [SC2086]

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.

3 participants