Skip to content
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

Preservation of OS images when re-runnning 'epicli apply' #2329

Merged
merged 23 commits into from
May 28, 2021
Merged

Preservation of OS images when re-runnning 'epicli apply' #2329

merged 23 commits into from
May 28, 2021

Conversation

seriva
Copy link
Collaborator

@seriva seriva commented May 18, 2021

  • Implemented a mechanism to use the current OS images for every OS and not only the default we have defined for Ubuntu.
  • Added option for preservation of OS images when re-runnning 'epicli apply' for existing cluster. In both cases a warning will be shown to preserve the images.
  • Removed commented out validation command is we will not be implementing that anymore in Epicli.

@seriva seriva changed the title Feature/2288 Preservation of OS images when re-runnning 'epicli apply' May 19, 2021
@przemyslavic
Copy link
Collaborator

/azp run

@przemyslavic
Copy link
Collaborator

/azp run

@przemyslavic
Copy link
Collaborator

Unit tests fail:

=================================== FAILURES ===================================
_____________ test_get_public_ip_should_set_proper_values_to_model _____________

    def test_get_public_ip_should_set_proper_values_to_model():
        cluster_model = get_cluster_model(cluster_name='TestCluster')
        builder = InfrastructureBuilder([cluster_model])

        component_value = dict_to_objdict({
            'machine': 'kubernetes-master-machine'
        })
>       vm_config = builder.get_virtual_machine(component_value, cluster_model, [], [], False)
E       TypeError: get_virtual_machine() takes 2 positional arguments but 6 were given

tests/engine/providers/azure/test_AzureConfigBuilder.py:71: TypeError
_________ test_get_network_interface_should_set_proper_values_to_model _________

    def test_get_network_interface_should_set_proper_values_to_model():
        cluster_model = get_cluster_model(cluster_name='TestCluster')
        builder = InfrastructureBuilder([cluster_model])
        component_value = dict_to_objdict({
            'machine': 'kubernetes-master-machine'
        })
>       vm_config = builder.get_virtual_machine(component_value, cluster_model, [], [], False)
E       TypeError: get_virtual_machine() takes 2 positional arguments but 6 were given

tests/engine/providers/azure/test_AzureConfigBuilder.py:87: TypeError
- generated xml file: /azp/agent/_work/6/s/core/src/epicli/tests/results/result.xml -
=========================== short test summary info ============================
FAILED tests/engine/providers/azure/test_AzureConfigBuilder.py::test_get_public_ip_should_set_proper_values_to_model
FAILED tests/engine/providers/azure/test_AzureConfigBuilder.py::test_get_network_interface_should_set_proper_values_to_model
=================== 2 failed, 61 passed, 2 skipped in 0.52s ====================

@przemyslavic
Copy link
Collaborator

/azp run

…nd not only the default we have defined for Ubuntu.
docs/home/howto/CLUSTER.md Outdated Show resolved Hide resolved
docs/home/howto/CLUSTER.md Outdated Show resolved Hide resolved
docs/home/howto/CLUSTER.md Outdated Show resolved Hide resolved
docs/home/howto/CLUSTER.md Outdated Show resolved Hide resolved
@seriva seriva marked this pull request as ready for review May 21, 2021 18:09
docs/home/howto/CLUSTER.md Outdated Show resolved Hide resolved
@seriva seriva requested a review from mkyc May 27, 2021 13:47
Copy link
Contributor

@mkyc mkyc left a comment

Choose a reason for hiding this comment

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

Shouldn't we update automated tests to ensure that os version didn't change in apply -> upgrade -> apply scenario? @przemyslavic can you help here? Do we have any automated tests for this? Where should we update?

@seriva
Copy link
Collaborator Author

seriva commented May 28, 2021

Shouldn't we update automated tests to ensure that os version didn't change in apply -> upgrade -> apply scenario? @przemyslavic can you help here? Do we have any automated tests for this? Where should we update?

Unit tests are updated, as the automated tests, that is probably more a pipeline thing which I think @przemyslavic was already setting up?

@przemyslavic
Copy link
Collaborator

przemyslavic commented May 28, 2021

As discussed during the standup, unit tests should be sufficient here. A pipeline for testing the LTS version has already been created.

docs/home/howto/CLUSTER.md Outdated Show resolved Hide resolved
docs/home/howto/CLUSTER.md Show resolved Hide resolved
docs/home/howto/UPGRADE.md Outdated Show resolved Hide resolved
@seriva seriva requested review from mkyc and to-bar May 28, 2021 10:22
to-bar
to-bar previously approved these changes May 28, 2021
@przemyslavic
Copy link
Collaborator

/azp run

docs/home/howto/CLUSTER.md Outdated Show resolved Hide resolved
docs/home/howto/CLUSTER.md Outdated Show resolved Hide resolved
docs/home/howto/CLUSTER.md Outdated Show resolved Hide resolved
@seriva seriva merged commit b3cfb53 into hitachienergy:develop May 28, 2021
sbbroot pushed a commit to sbbroot/epiphany that referenced this pull request Aug 17, 2021
…ergy#2329)

- Implemented a mechanism to use the current OS images for every OS and not only the default we have defined for Ubuntu.
- Added option for preservation of OS images when re-runnning 'epicli apply' for existing cluster. In both cases a warning will be shown to preserve the images.
- Removed commented out validation command is we will not be implementing that anymore in Epicli.
@seriva seriva deleted the feature/2288 branch October 18, 2021 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants