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.

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

Jump to bottom

Add istio.io repo #9983

Merged
merged 5 commits into from
Jan 2, 2019
Merged

Add istio.io repo #9983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9acc85f
Add isito.io repo
clyang82 Nov 15, 2018
84ff5bf
Add monthly flag to replace
clyang82 Nov 21, 2018
7317c11
address review comments
clyang82 Nov 28, 2018
1fa6bef
move modify helm values into modify_values.sh
clyang82 Nov 30, 2018
ee90930
add back missed changes
clyang82 Nov 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions install/kubernetes/helm/istio/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,11 @@ The chart deploys pods that consume minimum resources as specified in the resour
EOF
```

1. Add `istio.io` chart repository and point to the release:
```
$ helm repo add istio.io https://storage.googleapis.com/istio-prerelease/daily-build/release-1.1-latest-daily/charts
Copy link
Contributor

Choose a reason for hiding this comment

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

What will be the URL for production users ? We can't use prerelease and storage.

Copy link
Member Author

Choose a reason for hiding this comment

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

it will be replaced during creating release package.

  if [ "${CB_PIPELINE_TYPE}" = "monthly" ]; then
    local origin_url="istio-prerelease/daily-build/master-latest-daily/charts"
    local target_url="istio-release/releases/${VER_STRING}/charts"
    sed -i.bak "s:${origin_url}:${target_url}:g" "${COMMON_FILES_DIR}/install/kubernetes/helm/istio/README.md"
    rm -rf "${COMMON_FILES_DIR}/install/kubernetes/helm/istio/README.md.bak"
  fi
}

Copy link
Member

Choose a reason for hiding this comment

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

@clyang82 Can you please add some comments to production users here? +1 to @costinm , it seems to be weird to use pre release here.

```

1. Build the Helm dependencies:
```
$ helm dep update install/kubernetes/helm/istio
Expand Down
1 change: 1 addition & 0 deletions prow/release-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ cat << EOF > "/workspace/gcb_env.sh"
export CB_BRANCH="${GIT_SHA}"
export CB_VERSION="${GIT_SHA}"
export CB_ISTIOCTL_DOCKER_HUB="docker.io/istio"
export CB_PIPELINE_TYPE=daily
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we need two tests for the two types?

Copy link
Member Author

@clyang82 clyang82 Nov 28, 2018 •
edited

Choose a reason for hiding this comment

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

for test coverage purpose, I think we should have two tests for the two types. but considering the 2 factors:

  1. there is no many differences for the two types right now.
  2. add one more test needs spend extra 13 minutes.
    Maybe we can consider in the future.
    @hklai @rkpagadala

EOF
}

Expand Down
21 changes: 16 additions & 5 deletions release/create_release_archives.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@
#
################################################################################

set -o errexit
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you move these down?

Copy link
Member Author

Choose a reason for hiding this comment

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

set -o errexit
set -o pipefail
set -x

Move them together to ensure perform this cmd create_release_archives.sh only show the usage without unnecessary information

Before changes:

chunlins-mbp:istio clyang$ ./release/create_release_archives.sh 
++ mktemp -d /tmp/istio.version.XXXX
+ TEMP_DIR=/tmp/istio.version.wORR
+ BASE_DIR=/tmp/istio.version.wORR
+ ISTIOCTL_SUBDIR=istioctl
+ OUTPUT_PATH=
+ VER_STRING=
+ getopts d:i:o:v: arg
+ [[ -z /tmp/istio.version.wORR ]]
+ [[ -z '' ]]
+ usage
+ echo './release/create_release_archives.sh
    -d <path> path to use for temp directory                  (optional, randomized default is /tmp/istio.version.wORR )
    -o <path> path where build output/artifacts are stored    (required)
    -i <name> subdirectory in -o path to use for istioctl     (optional)
    -v <ver>  version info to include in filename (e.g., 1.0) (required)'
./release/create_release_archives.sh
    -d <path> path to use for temp directory                  (optional, randomized default is /tmp/istio.version.wORR )
    -o <path> path where build output/artifacts are stored    (required)
    -i <name> subdirectory in -o path to use for istioctl     (optional)
    -v <ver>  version info to include in filename (e.g., 1.0) (required)
+ exit 1

After Changes:

chunlins-mbp:istio clyang$ ./release/create_release_archives.sh 
./release/create_release_archives.sh
    -d <path> path to use for temp directory                  (optional, randomized default is /tmp/istio.version.XXXX )
    -o <path> path where build output/artifacts are stored    (required)
    -i <name> subdirectory in -o path to use for istioctl     (optional)
    -v <ver>  version info to include in filename (e.g., 1.0) (required)

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's ignore those for now. This file is not mean to be used by human directly anyway, and @rkpagadala is moving stuff around.

I suggest we just revert.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hklai If we can make it use by human directly, why not. At least, when I did test the release package, I need to run this Shell to generate the package. Then show the usage more friendly is not so bad. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK it's fine to make it more usable.

set -o pipefail
set -x

# This script primarily exists for Cloud Builder. This script
# reads artifacts from a specified directory, generates tar files
# based on those artifacts, and then stores the tar files
Expand All @@ -30,6 +26,13 @@ ISTIOCTL_SUBDIR=istioctl
OUTPUT_PATH=""
VER_STRING=""

function cleanup() {
rm -rf "$TEMP_DIR"
}

# do cleanup before the script exits
trap cleanup EXIT

function usage() {
echo "$0
-d <path> path to use for temp directory (optional, randomized default is ${BASE_DIR} )
Expand All @@ -45,6 +48,11 @@ function error_exit() {
exit "${2:-1}"
}

# since there are 2 required options, should show usage and exit with no args specified
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the following does exactly that?

if (($# == 0)); then
usage
fi

while getopts d:i:o:v: arg ; do
case "${arg}" in
d) BASE_DIR="${OPTARG}";;
Expand All @@ -55,6 +63,10 @@ while getopts d:i:o:v: arg ; do
esac
done

set -o errexit
set -o pipefail
set -x

[[ -z "${BASE_DIR}" ]] && usage
[[ -z "${OUTPUT_PATH}" ]] && usage
[[ -z "${VER_STRING}" ]] && usage
Expand Down Expand Up @@ -159,4 +171,3 @@ create_osx_archive
create_windows_archive
popd

rm -rf "$TEMP_DIR"
9 changes: 8 additions & 1 deletion release/gcb/helm_values.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,21 @@ function fix_values_yaml() {
# Copy helm charts (build by helm_charts.sh) to be packaged in the tarball.
mkdir -vp ./"istio-${CB_VERSION}"/install/kubernetes/helm/charts
cp /workspace/charts/* ./"istio-${CB_VERSION}"/install/kubernetes/helm/charts

# replace prerelease with release location for istio.io repo
if [ "${CB_PIPELINE_TYPE}" = "monthly" ]; then
sed -i.bak "s:istio-prerelease/daily-build.*$:istio-release/releases/${CB_VERSION}/charts:g" ./"istio-${CB_VERSION}"/install/kubernetes/helm/istio/README.md
rm -rf ./"istio-${CB_VERSION}"/install/kubernetes/helm/istio/README.md.bak
echo "Done replacing pre-released charts with released charts for istio.io repo"
fi

eval "$zip_cmd" "${tarball_name}" "istio-${CB_VERSION}"
sha256sum "${tarball_name}" > "${tarball_name}.sha256"
rm -rf "istio-${CB_VERSION}"

gsutil -q cp "${tarball_name}" "gs://${CB_GCS_BUILD_PATH}/${tarball_name}"
gsutil -q cp "${tarball_name}.sha256" "gs://${CB_GCS_BUILD_PATH}/${tarball_name}.sha256"
echo "DONE fixing gs://${CB_GCS_BUILD_PATH}/${tarball_name} with hub: ${CB_DOCKER_HUB} tag: ${CB_VERSION}"
echo "DONE fixing gs://${CB_GCS_BUILD_PATH}/${tarball_name} with hub: ${CB_DOCKER_HUB} tag: ${CB_VERSION}"
}

mkdir -p modification-tmp
Expand Down