Refeactored framework deployment utils

[alejandrox1]

xref: #76206

Signed-off-by: Jorge Alarcon Ochoa alarcj137@gmail.com

What type of PR is this?

/kind cleanup

What this PR does / why we need it:
Refactor and cleanup e2e framework deployment utils
xref: #76206

Special notes for your reviewer:
This is intended to be the beginning of the refactoring of framework/deployment_utils.go into framework/deployment.

Does this PR introduce a user-facing change?:

NONE

[alejandrox1]

/area e2e-test-framework
/area test
/sig testing

[cblecker]

/uncc
Don't have the bandwidth to review this at the moment.

/cc @andrewsykim
Do you have specific reviewers helping with this effort, Andrew?

[neolit123]

LGTM @alejandrox1
if you are ready with this we can remove the WIP.

first run ./hack/update-bazel.sh
and include the resulted changes using git add...

then look at these failures:

test/e2e/apps/deployment.go:324:20: undefined: framework.UpdateDeploymentWithRetries
test/e2e/apps/deployment.go:466:20: undefined: framework.UpdateDeploymentWithRetries
test/e2e/apps/deployment.go:539:21: undefined: framework.UpdateDeploymentWithRetries
test/e2e/apps/deployment.go:685:22: undefined: framework.UpdateDeploymentWithRetries
test/e2e/apps/deployment.go:695:22: undefined: framework.UpdateDeploymentWithRetries
test/e2e/apps/deployment.go:706:22: undefined: framework.UpdateDeploymentWithRetries
test/e2e/apps/deployment.go:715:23: undefined: framework.UpdateDeploymentWithRetries
test/e2e/apps/deployment.go:722:23: undefined: framework.UpdateDeploymentWithRetries
test/e2e/apps/deployment.go:759:21: undefined: framework.UpdateDeploymentWithRetries
test/e2e/apps/deployment.go:865:20: undefined: framework.UpdateDeploymentWithRetries
test/e2e/apps/deployment.go:865:20: too many errors
# k8s.io/kubernetes/test/e2e/upgrades/apps
test/e2e/upgrades/apps/deployments.go:86:20: undefined: framework.UpdateDeploymentWithRetries
test/e2e/upgrades/apps/deployments.go:162:11: undefined: framework.UpdateDeploymentWithRetries

try building the suite locally with those commands i gave you on slack.
when done git add all fixes and force push your commit to the remote branch.

[neolit123]

/unassign timothysc
until WIP is done.

[alejandrox1]

Thank you so much for all of your comments @neolit123 I'm feeling a lot more comfortable getting into the framework.

I opened this pr based on the comments of #76800. Please tell me if it makes more sense to do multiple small PRs to refactor framework/deployment_util.go into framework/deployment or if it would be better for me to try and do it all at once. My thinking is that small changes are easier to review but it may not be such a big deal for others who are a lot more familiar with the code.

[neolit123]

thank you for the update @alejandrox1

I opened this pr based on the comments of #76800. Please tell me if it makes more sense to do multiple small PRs to refactor framework/deployment_util.go into framework/deployment or if it would be better for me to try and do it all at once. My thinking is that small changes are easier to review but it may not be such a big deal for others who are a lot more familiar with the code.

i think this current PR is a good example in terms of size vs contents.
PRs like this one are preferred instead of many lines of changes.

thanks
/priority backlog
/approve

[neolit123]

/lgtm
/assign @timothysc

[oomichi]

@alejandrox1

Hello,

I opened this pr based on the comments of #76800. Please tell me if it makes more sense to do multiple small PRs to refactor framework/deployment_util.go into framework/deployment or if it would be better for me to try and do it all at once. My thinking is that small changes are easier to review but it may not be such a big deal for others who are a lot more familiar with the code.

Does the above comment mean we will have more PRs which move all remaining code of test/e2e/framework/deployment_util.go into a new path?
When I checked this PR at the first look, I was confused because this PR just moves one function.
It is better to mention about that on the PR message which mean we have more TODOs.

The code itself seems good for me anyways.

[alejandrox1]

Hi @oomichi

Does the above comment mean we will have more PRs which move all remaining code of test/e2e/framework/deployment_util.go into a new path?
When I checked this PR at the first look, I was confused because this PR just moves one function.
It is better to mention about that on the PR message which mean we have more TODOs.

Thank you for your comment. I added some more context into the PR message.
I wanted to begin with one just one function to get some feedback on whether this is would be the optimal way of refactoring framework/deployment_util.go and I was also doubtful about the appropriate number of changes that I should commit in order to make reviewing easy.

[timothysc]

what about all the other functions under deployment_util?

[timothysc]

meh, I'll approve and keep on trucking.

However I'd prefer in the future to bundle shuffling together in complete units.

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alejandrox1, neolit123, timothysc

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

The pull request process is described here

Needs approval from an approver in each of these files:

• test/OWNERS [timothysc]

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