-
Notifications
You must be signed in to change notification settings - Fork 113
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
e2e: Use helm
to deploy on k8s cluster
#704
Conversation
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 9674487218Details
💛 - Coveralls |
Thanks for your PR,
To skip the vendors CIs use one of:
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
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.
great work!
chmod 700 $(BIN_DIR)/get_helm.sh | ||
|
||
$(BIN_DIR)/helm: $(BIN_DIR)/get_helm.sh | ||
HELM_INSTALL_DIR=$(BIN_DIR) $(BIN_DIR)/get_helm.sh |
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.
WDYT about moving the logic from above get_helm.sh here and deleting the get_helm.sh file at the end.
so we remain just with helm binary ?
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.
Makes sense. Cleaning
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.
ideally we should have a $(BIN_DIR) target which creates bin directroy then have this target depend on it.
same goes for other targets that download/install bin dependencies
that said its not related to this PR
Thanks for your PR,
To skip the vendors CIs use one of:
|
make helm | ||
helm install -n ${NAMESPACE} --create-namespace \ | ||
$HELM_VALUES_OPTS \ | ||
--wait sriov-network-operator ./deployment/sriov-network-operator |
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.
FYI #706 will rename the chart to sriov-network-operator-chart.
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.
Ack!
Thanks for your PR,
To skip the vendors CIs use one of:
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
@zeeke can you rebase this one ? |
Thanks for your PR,
To skip the vendors CIs use one of:
|
envsubst< $root/deploy/namespace.yaml | ${OPERATOR_EXEC} apply -f - | ||
|
||
HELM_VALUES_OPTS="\ | ||
--set images.operator=${SRIOV_NETWORK_OPERATOR_IMAGE} \ |
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.
hmm ci falling.
maybe need to quote the images ? have a feeling its something with helm templating.
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.
We actually caught a regression introduced by
this is already paying back its effort
Thanks for your PR,
To skip the vendors CIs use one of:
|
lets have CI green and we can merge imo |
Using `helm` to deploy the operator in vanilla k8s CI lanes helps avoid error in helm charts changes. Instead of adding a new helm e2e lane (as it would increase the CI load), we can keep the already in place lanes as: - OpenShift: use the `hack/deploy-setup.sh` script - K8s: use helm chart Signed-off-by: Andrea Panattoni <[email protected]>
Signed-off-by: Andrea Panattoni <[email protected]>
Thanks for your PR,
To skip the vendors CIs use one of:
|
CI green, got two approvals. |
Using
helm
to deploy the operator in vanilla k8s CI lanes helps avoid error in helm charts changes.Instead of adding a new helm e2e lane (as it would increase the CI load), we can keep the already in place lanes as:
hack/deploy-setup.sh
script