-
Notifications
You must be signed in to change notification settings - Fork 2
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diverdane Just a few small comment but great investigative work on this. We may need to consider bumping the version right after this commit (or as part of it).
56058b7
to
b8be637
Compare
@sgnn7 - I made the changes that you suggested.
|
f40a84a
to
a096a75
Compare
kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/marketplace-k8s-app-tools/master/crd/app-crd.yaml" | ||
# Ignore errors on kubectl apply. `AlreadyExists` Errors can occur if | ||
# another parallel test is doing a kubectl apply at the same time. | ||
-kubectl apply -f "https://raw.githubusercontent.com/GoogleCloudPlatform/marketplace-k8s-app-tools/master/crd/app-crd.yaml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this is a risky move but maybe it will be fine. I'll leave it as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I did not like adding this, but the first time I ran the 2 test cases in parallel, I saw an AlreadyExists
error in one test. I didn't expect to see this sort of error when kubectl apply ...
is being used, but there's apparently a race condition where the 2 instances of kubectl apply ...
realize that a create is needed, and they both try the create at the same time. It's too bad there's no --ignore-already-created flag or similar to whatever the flag is for kubectl create ...
.
ci/secrets.yml
Outdated
GCLOUD_ZONE: !var ci/google-conjur-cloud-launcher-onboard/gcloud-zone | ||
|
||
gke-1-15-9: | ||
GCLOUD_CLUSTER_NAME: onboarding-cluster-1-15-9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pending resolution of convo on slack
README.md
Outdated
@@ -89,7 +89,7 @@ export NAMESPACE=conjur | |||
Configure the container images: | |||
|
|||
```shell | |||
export TAG_VERSION=1.3.4 | |||
export TAG_VERSION=1.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think pinning to full semver version might be better than to the minor version since you won't be able to know what version of components you're pulling/writing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, there are no tags using the full semver version in our registry: https://console.cloud.google.com/gcr/images/cloud-marketplace/GLOBAL/cyberark/conjur-open-source?gcrImageListsize=30
We can change this when we make a new release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I switched this to the full semver 1.4.0
in anticipation of us making a release 1.4.0
and then publishing it to Google Cloud Marketplace.
1004060
to
998e8a2
Compare
This change fixes a problem whereby deployment of the marketplace app would fail on GKE clusters with Kubernetes versions >= 1.15. The main problem was that we were using the Google Marketplace Tools Version 0.7.0, and this uses a version of kubectl (1.12) that does not know how to authenticate with Kubernetes version 1.15 or newer. The changes included are as follows: * Upgraded the Google Marketplace Tools container image used by the Deployer from 0.7.0 to 0.10.0. The newer version of these tools provide an adaptive Kubectl client version (tools read the Kubernetes server version, then select a matching kubectl binary). * Upgraded the Google Marketplace Tools submodule to 0.10.0. * Upgraded the version of Helm used by the deployer from 2.6.1 to 2.16.1 to eliminate this Helm bug: helm/helm#2998 * Deleted x-google-marketplace section for tester.image in schema.yaml to be consistent for Google Marketplace Tools v0.10.0 * Added a build.sh flag (`-p` or `--persist`) to persist the application deployment after testing. * Fixed the deployment's `deploy-info` annotations to use keys that are valid JSON keys (i.e. with quotes). Fixes Issue #25 "Project fails to deploy on GKE 1.15.7-gke.23"
998e8a2
to
67e998d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diverdane Great work on this! LGTM!
This change fixes a problem whereby deployment of the marketplace
app would fail on GKE clusters with Kubernetes versions >= 1.15.
The main problem was that we were using the Google Marketplace Tools
Version 0.7.0, and this uses a version of kubectl (1.12) that does
not know how to authenticate with Kubernetes version 1.15 or newer.
The changes included are as follows:
-p
or--persist
) to persist the application deployment after testing.deploy-info
annotations to use keys that are valid JSON keys (i.e. with quotes).Fixes Issue #25 "Project fails to deploy on GKE 1.15.7-gke.23"
Fixes Issue #26 "GCP marketplace integration has automated tests against the latest GKE versions"