Skip to content
This repository was archived by the owner on Jun 14, 2019. It is now read-only.

Conversation

@vrutkovs
Copy link

@vrutkovs vrutkovs commented Oct 10, 2018

Related to openshift/release#1878 and openshift/release#1866

This also adds new test types - openshift-ansible upgrade and custom openshift-ansible provisioners.

Fixes #175

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 10, 2018
@vrutkovs vrutkovs force-pushed the cluster-types-gluster-logging branch from 919cfbb to 8dd26de Compare October 10, 2018 17:37
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 10, 2018
@droslean
Copy link
Member

/hold
You should update also the validations.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 10, 2018
@bbguimaraes
Copy link
Contributor

Since you're doing this, you can also add the test types for #175. The existing test types are here:

ContainerTestConfiguration *ContainerTestConfiguration `json:"container,omitempty"`
OpenshiftAnsibleClusterTestConfiguration *OpenshiftAnsibleClusterTestConfiguration `json:"openshift_ansible,omitempty"`
OpenshiftAnsibleSrcClusterTestConfiguration *OpenshiftAnsibleSrcClusterTestConfiguration `json:"openshift_ansible_src,omitempty"`
OpenshiftInstallerClusterTestConfiguration *OpenshiftInstallerClusterTestConfiguration `json:"openshift_installer,omitempty"`

Once they are valid ci-operator test types, we can change the generator to support those templates.

@vrutkovs vrutkovs force-pushed the cluster-types-gluster-logging branch from 8dd26de to 92d1eb3 Compare October 11, 2018 09:05
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 11, 2018
CONFIGURATION.md Outdated
target_cloud: ''
openshift_ansible_upgrade:
target_cloud: ''
previous_ansible_version: ''
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like it could mirror tag_specification and fully define the previous release, so you don't need to explicitly provide the version, image and RPMs

Copy link
Author

Choose a reason for hiding this comment

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

It seems previous_version and previous_rpm_deps should be sufficient

@vrutkovs vrutkovs force-pushed the cluster-types-gluster-logging branch 3 times, most recently from b234481 to 2c8d76b Compare October 11, 2018 09:23
CONFIGURATION.md Outdated
openshift_ansible_src:
target_cloud: ''
openshift_ansible_upgrade:
target_cloud: ''
Copy link
Author

Choose a reason for hiding this comment

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

Not sure why the docs state this field is target_cloud and the class json field is cluster_profile. Docs issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

#178 = )

Copy link
Contributor

Choose a reason for hiding this comment

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

I blame the happy hour.

@vrutkovs vrutkovs force-pushed the cluster-types-gluster-logging branch from 2c8d76b to a1c6330 Compare October 11, 2018 09:49
@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2018
@vrutkovs vrutkovs force-pushed the cluster-types-gluster-logging branch from a1c6330 to 40ad537 Compare October 11, 2018 09:54
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2018
needsReleaseRpms = true
validationErrors = append(validationErrors, validateClusterProfile(fmt.Sprintf("%s", fieldRoot), testConfig.ClusterProfile))
}
if testConfig := test.OpenshiftAnsibleUpgradeClusterTestConfiguration; testConfig != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do similar for custom?

Copy link
Author

Choose a reason for hiding this comment

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

Done

@vrutkovs vrutkovs force-pushed the cluster-types-gluster-logging branch from d8392b6 to ac9af1d Compare October 11, 2018 10:27
@stevekuznetsov
Copy link
Contributor

/unassign
/assign @bbguimaraes

pkg/api/types.go Outdated
ClusterProfile ClusterProfile `json:"cluster_profile"`
PreviousVersion string `json:"previous_version"`
PreviousRPMDeps string `json:"previous_rpm_deps"`
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like you need this separate struct. These fields can be added to OpenshiftAnsibleUpgradeClusterTestConfiguration directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Because clusters are always upgraded from one version to the next, these could also be derived automatically from tag_specification.

Copy link
Author

Choose a reason for hiding this comment

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

Deriving previous version is fairly complicated, 4.0 -> 3.11 (it could as well be 3.12 -> 3.11), previous RPM deps repo is not easy too

Copy link
Author

Choose a reason for hiding this comment

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

Additional struct is for this is no longer created

@bbguimaraes
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 12, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bbguimaraes, vrutkovs

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

The pull request process is described 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2018
@bbguimaraes
Copy link
Contributor

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 12, 2018
@openshift-merge-robot openshift-merge-robot merged commit 1cac8bc into openshift:master Oct 12, 2018
@vrutkovs vrutkovs deleted the cluster-types-gluster-logging branch May 16, 2019 11:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants