[WIP] Update cluster init before upgrade #1110
Conversation
mjura
changed the title
| @@ -50,18 +50,20 @@ def cluster_deploy(self, kubernetes_version=None, cloud_provider=None, | |||
|
|
|||
|
|
|||
| @step | |||
| def cluster_init(self, kubernetes_version=None, cloud_provider=None): | |||
| def cluster_init(self, kubernetes_version=None, cloud_provider=None, path=None): | |||
There was a problem hiding this comment.
We should add the path parameter also to the testrunner command in both the bootstrap and deploy commands, which call cluster_init https://github.com/SUSE/skuba/blob/master/ci/infra/testrunner/testrunner.py#L167-L181 and https://github.com/SUSE/skuba/blob/master/ci/infra/testrunner/testrunner.py#L43-L56
And pass it when invoking the cluster init here https://github.com/SUSE/skuba/blob/master/ci/infra/testrunner/skuba/skuba.py#L43-L49 and here https://github.com/SUSE/skuba/blob/master/ci/infra/testrunner/skuba/skuba.py#L71-L74.
ci/infra/testrunner/skuba/skuba.py
Outdated
| @@ -154,6 +156,10 @@ def node_remove(self, role="worker", nr=0): | |||
| def node_upgrade(self, action, role, nr, ignore_errors=False): | |||
| self._verify_bootstrap_dependency() | |||
|
|
|||
| self.cluster_init("", "", "/tmp") | |||
There was a problem hiding this comment.
I'm worried if this is a one-time fix. That is, IIRC this applies only when upgrading from cri-0 1.17 to 1.18 but not in other cases.
ci/infra/testrunner/skuba/skuba.py
Outdated
| @@ -223,3 +229,18 @@ def _run_skuba(self, cmd, cwd=None, verbosity=None, ignore_errors=False): | |||
| cwd=cwd, | |||
| ignore_errors=ignore_errors, | |||
| ) | |||
|
|
|||
| def _run_os_cmd(self, cmd, cwd=None, ignore_errors=False): | |||
There was a problem hiding this comment.
I only see you are using this _run_os_cmd once. Any reason for not calling runshlelcommand directly?
ci/infra/testrunner/skuba/skuba.py
Outdated
| @@ -154,6 +156,10 @@ def node_remove(self, role="worker", nr=0): | |||
| def node_upgrade(self, action, role, nr, ignore_errors=False): | |||
| self._verify_bootstrap_dependency() | |||
|
|
|||
| self.cluster_init("", "", "/tmp") | |||
| cmd = "rsync -a /tmp/test-cluster/addons/ addons/" | |||
There was a problem hiding this comment.
I don't like this hardcoded path for several reasons. First, In the CI, for example it is unsafe to use /tmp. I would be safer to create a tmp directory in the workspace directory.
Also, instead of hardcoding test-cluster you could better use the configuration parameter self.config.skuba.cluster because the cluster_init function will use it.
|
related to #1251 |
|
it is not needed any more |
Why is this PR needed?
For migration between
CaaSP 4.2.1andCaaSP 4.3cluster addons directory should be updated. It was already covered by release noteshttps://github.com/SUSE/caasp-release-notes/pull/51/files
This has to be also covered by our CI framework
What does this PR do?
Merge restrictions
(Please do not edit this)
We are in v4-maintenance phase, so we will restrict what can be merged to prevent unexpected surprises: