Skip to content

Add kubetest2 deployer for e2e testing - #1510

Draft
breuerfelix wants to merge 8 commits into
mainfrom
kubetest2-init
Draft

Add kubetest2 deployer for e2e testing#1510
breuerfelix wants to merge 8 commits into
mainfrom
kubetest2-init

Conversation

@breuerfelix

Copy link
Copy Markdown
Member

How to categorize this PR?

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Breaking changes:

/hold still WIP

@ske-prow

ske-prow Bot commented Aug 19, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ske-prow ske-prow Bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Aug 19, 2026
@ske-prow

ske-prow Bot commented Aug 19, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jamand for approval. For more information see the Code Review Process.

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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ske-prow ske-prow Bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 19, 2026
Comment thread test/kubetest2/deployer.go Outdated
Comment thread test/kubetest2/bootstrap_clients.go Outdated
Comment thread test/kubetest2/lifecycle.go Outdated
klog.Infof("Starting cluster up flow for cluster=%q", d.clusterName())

ctx := context.Background()
if err := d.ensureManagedClusterAccess(ctx); err != nil {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

somehow i don't like that this is split up in "bootstrap" and "ske cluster provisioning". To me all steps are equally important in terms of bootstrapping, also deploying the csi helmchart will be the last step.
i would like to organize this a little different.
add another folder, lets name it "bootstrap" if you will, inside there are multiple go files for every step.
project.go
serviceaccount.go
ske.go
csi.go
...
Each expose an ensure function. "ensureProject", "ensureCluster" etc.
The Up() function just calls these functions one after another :) and each function itself is idempotent.

This would overall help with getting through the code and understanding when which step is happening and each step can be encapsulated really nice :)

WDYT?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Im going with a flattened/restructured file list for now. A bootstrap/ folder would result in a new go package, so ensureProject etc. couldn't stay methods on *Deployer and we would have to refactor a lot. The bootstrap_*.go prefix gives the same readability IMO. What do you think?

@ske-prow ske-prow Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants