-
Notifications
You must be signed in to change notification settings - Fork 38
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
Updated kit pipelines for kubeconfig workspaces #385
base: main
Are you sure you want to change the base?
Conversation
This commit updates kit pipelines for kubeconfig workspaces. Signed-off-by: Ashish Ranjan <[email protected]>
@@ -0,0 +1,40 @@ | |||
apiVersion: tekton.dev/v1beta1 | |||
kind: PipelineRun |
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.
Keep this pipelinerun here will be automatically applied to Infra cluster due to FLUX sync. Can we move this example outside of tests dir ?
- name: config | ||
workspace: config | ||
|
||
# finally: |
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.
why is this commented out ?
mountPath: /config/ | ||
steps: | ||
- name: setup-control-plane | ||
image: bitnami/kubectl:1.24.5 # curl was removed in more recent versions |
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.
can't we use Alpine regular image that we use in other tasks ?
kubectl create namespace $(params.cluster-name) | ||
fi | ||
echo "Setting up control plane" | ||
cat <<EOF > /tmp/controlplane.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.
let's take these files as params which will reduce lot's of params in these tasks and boils down to file locations, it's much cleaner that way and also it's much more dynamic than embedding it here. Example - like how we take it for service role tasks etc.
# sanity check to ensure it's live. | ||
kubectl --kubeconfig $(workspaces.config.path)/kubeconfig version | ||
- name: install-cni | ||
image: bitnami/kubectl |
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.
same
dataplane_name=$dp_name-$1 | ||
EC2_INSTANCES=$3 | ||
cat <<EOF >> /tmp/dp.yaml | ||
apiVersion: kit.k8s.sh/v1alpha1 |
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.
let's not embed these things and give flexibility to user to pass in as file location be it github or s3 whatever makes most sense.
metadata: | ||
name: label-and-taint-node | ||
namespace: tekton-pipelines | ||
annotations: |
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 don't think we need this annotations. it's hard to maintain as we change versions etc. Let's keep it simple and cut down as much or trim it to zero if possible.
- name: KUBECONFIG | ||
value: /config/kubeconfig | ||
steps: | ||
- name: setup-data-plane |
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.
step name doesn't make sense for the task. Also, can we not have this as separate task and add it as a step to setup task ? Or do we have any reason to separate out this as it's own Task ?
Also can we close this draft PR then #352 as you are covering it here ? |
This commit updates kit pipelines for kubeconfig workspaces.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.