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

Create artifacts to given directory #581

Merged
merged 5 commits into from
Aug 21, 2017

Conversation

chxchx
Copy link
Contributor

@chxchx chxchx commented Aug 18, 2017

Add a new flag to release/create_release_archives.sh so tar files are made in the given directory, which makes release automation easier as per istio/test-infra#393

Release note:

None

@chxchx
Copy link
Contributor Author

chxchx commented Aug 18, 2017

/assign @rshriram
/assign @sebastienvas
PTAL

TMP_DIR="$(mktemp -d /tmp/istio.version.XXXX)"
COMMON_FILES_DIR="${TMP_DIR}/istio/istio-${ISTIO_VERSION}"
ARCHIVES_DIR="${TMP_DIR}/archives"
BASE_DIR="$(mktemp -d /tmp/istio.version.XXXX)"
Copy link
Contributor

@sebastienvas sebastienvas Aug 18, 2017

Choose a reason for hiding this comment

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

Here you are creating a temp dir. You should created it only if value is not net. ie
BASE_DIR=

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. I overlooked the fact that a directory is actually created besides returning a path. Good catch.

d) BASE_DIR="${OPTARG}";;
esac
done

Copy link
Contributor

Choose a reason for hiding this comment

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

Here you would add
[[ -z ${BASE_DIR} ]] && BASE_DIR="$(mktemp -d /tmp/istio.version.XXXX)"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@sebastienvas sebastienvas left a comment

Choose a reason for hiding this comment

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

PTAL

@chxchx
Copy link
Contributor Author

chxchx commented Aug 18, 2017

PTAL

@sebastienvas
Copy link
Contributor

/lgtm
/approve

@chxchx
Copy link
Contributor Author

chxchx commented Aug 21, 2017

/assign @rshriram
PTAL

@rshriram
Copy link
Member

/approve
/lgtm

@istio-merge-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rshriram, sebastienvas

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@istio-merge-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@istio-merge-robot
Copy link

Automatic merge from submit-queue

@istio-merge-robot istio-merge-robot merged commit 85326e0 into istio:master Aug 21, 2017
@istio-testing
Copy link
Collaborator

@chxchx: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/e2e-suite-rbac-auth.sh dcf642d link ``
prow/e2e-suite-no_rbac-auth.sh dcf642d link ``

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

mandarjog pushed a commit to mandarjog/istio that referenced this pull request Oct 30, 2017
* Add an adapter that implements every aspect but does no work.

* Add in an empty params proto so we don't run into issues returning nil

* Dump the empty params proto in favor of using proto.Empty. Add a test for verifying that the no-op adapter is registered for every aspect kind.

* Return a failed precondition in the noop Deny aspect impl.


Former-commit-id: 7ff9e1a200b431da305c89833773e825623bea8a
rshriram pushed a commit that referenced this pull request Oct 30, 2017
Automatic merge from submit-queue

Create artifacts to given directory

Add a new flag to `release/create_release_archives.sh` so tar files are made in the given directory, which makes release automation easier as per istio/test-infra#393

**Release note**:

```release-note
None
```

Former-commit-id: 85326e0
mandarjog pushed a commit that referenced this pull request Oct 31, 2017
* Add an adapter that implements every aspect but does no work.

* Add in an empty params proto so we don't run into issues returning nil

* Dump the empty params proto in favor of using proto.Empty. Add a test for verifying that the no-op adapter is registered for every aspect kind.

* Return a failed precondition in the noop Deny aspect impl.


Former-commit-id: d3b78618a13a69cdea71066f9f5f817afb4e74e8
vbatts pushed a commit to vbatts/istio that referenced this pull request Oct 31, 2017
Automatic merge from submit-queue

Create artifacts to given directory

Add a new flag to `release/create_release_archives.sh` so tar files are made in the given directory, which makes release automation easier as per istio/test-infra#393

**Release note**:

```release-note
None
```

Former-commit-id: 85326e0
mandarjog pushed a commit that referenced this pull request Oct 31, 2017
* validate proxy mesh config

* add auth policy validation

* simplify linter

* add host validation
mandarjog pushed a commit that referenced this pull request Nov 2, 2017
Automatic merge from submit-queue

Create artifacts to given directory

Add a new flag to `release/create_release_archives.sh` so tar files are made in the given directory, which makes release automation easier as per istio/test-infra#393

**Release note**:

```release-note
None
```

Former-commit-id: 85326e0
rshriram pushed a commit to rshriram/istio that referenced this pull request Jul 31, 2018
howardjohn pushed a commit to howardjohn/istio that referenced this pull request Jan 12, 2020
luksa added a commit to luksa/istio that referenced this pull request Sep 20, 2022
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

6 participants