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

Add commit hash as suffix to the OpenStackVersion #1217

Merged

Conversation

stuggi
Copy link
Contributor

@stuggi stuggi commented Dec 2, 2024

Right now when a PR merges, the build openstack-operator has the OpenStackVersion 0.0.1. Since they are all the same for each PR it is not possible to test updates. Lets add the commit hash as a suffix so that we get a different OpenStackVersion with each operator build.

Also adds a condition MinorUpateAvailable, which reflects that there is an update available. This can be used e.g. in CI jobs to use an oc wait command to wait when the new operator version got installed and the openstackversion is actually updated to reflect the new release.

@openshift-ci openshift-ci bot requested review from fao89 and lewisdenny December 2, 2024 16:41
@openshift-ci openshift-ci bot added the approved label Dec 2, 2024
@stuggi stuggi marked this pull request as draft December 2, 2024 16:41
stuggi added a commit to stuggi/install_yamls that referenced this pull request Dec 2, 2024
…target

* The openstack_ga target allows to deploy the oldest available
index of the openstack-operator using the commit from the
stable branch configured using the OPENSTACK_STABLE_BRANCH
environment variable, which defaults to 18.0-fr1.

* The openstack_crds_cleanup target allows to remove all openstack related
CRDs from an environment. All deployments need to be deleted before

* The openstack_deploy_update target batches the OpenstackVersion
target version to be the avaliable version to trigger an update.

With this an operator update from oldest available openstack-operator
index of the 18.0-fr1 branch to the latest from main can be tested
using this procedure:

rm -rf  out/operator/openstack-operator/
make openstack_ga
make openstack_deploy (wait for the deployment to finish)
make openstack_cleanup
make openstack
make openstack_deploy_update

Related: openstack-k8s-operators/openstack-operator#1217

Signed-off-by: Martin Schuppert <mschuppert@redhat.com>
@stuggi stuggi requested review from dprince and removed request for lewisdenny and fao89 December 2, 2024 16:44
stuggi added a commit to stuggi/install_yamls that referenced this pull request Dec 4, 2024
* The openstack_crds_cleanup target allows to remove all openstack
related CRDs from an environment. All deployments need to be
deleted before.

* The openstack_patch_version target patches the OpenstackVersion
target version to be the avaliable version to trigger an update.

With this an operator update (reinstall) and a deployment update
can be tested using this procedure:

~~~
make openstack_crds_cleanup
OPENSTACK_IMG=quay.io/openstack-k8s-operators/openstack-operator-index:123 make openstack_wait
TIMEOUT=600s make openstack_wait_deploy
make openstack_cleanup
make openstack_wait
make openstack_patch_version
~~~

TODO:
add targets to perform edpm deploy/update (ovn controller update
and final service update)

Depends-On: openstack-k8s-operators/openstack-operator#1217

Signed-off-by: Martin Schuppert <mschuppert@redhat.com>
@stuggi stuggi marked this pull request as ready for review December 4, 2024 15:38
@openshift-ci openshift-ci bot requested review from abays and bshephar December 4, 2024 15:38
Makefile Outdated
@@ -5,6 +5,8 @@
# - use environment variables to overwrite this value (e.g export VERSION=0.0.2)
VERSION ?= 0.0.1

OPENSTACK_RELEASE_VERSION ?= $(VERSION)-$(shell git rev-parse --short HEAD)
Copy link
Contributor

Choose a reason for hiding this comment

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

This may cause issues in some build environments. If I recall downstream build environments don't have git installed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we just use a timestamp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also for the Makefile OPENSTACK_RELEASE_VERSION its just the default, you can set to whatever is required downstream if the Makefile is used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed it to be the epoch

@@ -301,6 +301,15 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
Log.Info("Setting DeployedVersion")
instance.Status.DeployedVersion = &instance.Spec.TargetVersion
}
if instance.Status.AvailableVersion != nil &&
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is possible for AvailableVersion to be nil here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@dprince
Copy link
Contributor

dprince commented Dec 4, 2024

I'm not sure we strictly need this new condition to implement upgrade testing. If we add it we'll have it on main, but not on the FR1 maintenance branch so we won't be able to use the job there.

It is enough to just check that the new catalog/index is deployed and that OpenStackVersion is in a ReadyCondition True state. Then if targetVersion != availableVersion you'd know there is an update (and this is shown clearly on the CLI too)

Right now when a PR merges, the build openstack-operator has the
OpenStackVersion 0.0.1. Since they are all the same for each PR
it is not possible to test updates. Lets add the seconds since
the Epoch as a suffix so that we get a different OpenStackVersion
with each operator build.

Signed-off-by: Martin Schuppert <mschuppert@redhat.com>
@stuggi
Copy link
Contributor Author

stuggi commented Dec 5, 2024

I'm not sure we strictly need this new condition to implement upgrade testing. If we add it we'll have it on main, but not on the FR1 maintenance branch so we won't be able to use the job there.

It is enough to just check that the new catalog/index is deployed and that OpenStackVersion is in a ReadyCondition True state. Then if targetVersion != availableVersion you'd know there is an update (and this is shown clearly on the CLI too)

having the condition makes it a lot easier, as you could see from my install_yamls pr. it would be a single simple oc wait command, instead of checking the ReadyCondition, which is not really useful as it is True already from the previous update. so we'd have to check the deployedVersion or targetVersion against the availableVersion with a loop. we could really simplify this with this condition. Right now I think the initial focus is to test against main. but yes for running this job against latest in FR1 we have to backport this, which I think is not an issue.

When there is a new AvailableVersion, which does not match the
Deployed version, this adds a condition to reflect this. This
can be used e.g. in CI to wait for the new AvailableVersion to
be reflected when the OpenStackVersion was patched. But could
be useful for other situations.

Signed-off-by: Martin Schuppert <mschuppert@redhat.com>
@stuggi
Copy link
Contributor Author

stuggi commented Dec 5, 2024

/retest

3 similar comments
@stuggi
Copy link
Contributor Author

stuggi commented Dec 6, 2024

/retest

@stuggi
Copy link
Contributor Author

stuggi commented Dec 6, 2024

/retest

@stuggi
Copy link
Contributor Author

stuggi commented Dec 7, 2024

/retest

@openshift-ci openshift-ci bot added the lgtm label Dec 9, 2024
Copy link
Contributor

openshift-ci bot commented Dec 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprince, stuggi

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:

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

@openshift-merge-bot openshift-merge-bot bot merged commit 559ee7c into openstack-k8s-operators:main Dec 10, 2024
8 checks passed
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/install_yamls that referenced this pull request Dec 21, 2024
* The openstack_crds_cleanup target allows to remove all openstack
related CRDs from an environment. All deployments need to be
deleted before.

* The openstack_patch_version target patches the OpenstackVersion
target version to be the avaliable version to trigger an update.

With this an operator update (reinstall) and a deployment update
can be tested using this procedure:

~~~
make openstack_crds_cleanup
OPENSTACK_IMG=quay.io/openstack-k8s-operators/openstack-operator-index:123 make openstack_wait
TIMEOUT=600s make openstack_wait_deploy
make openstack_cleanup
make openstack_wait
make openstack_patch_version
~~~

TODO:
add targets to perform edpm deploy/update (ovn controller update
and final service update)

Depends-On: openstack-k8s-operators/openstack-operator#1217

Signed-off-by: Martin Schuppert <mschuppert@redhat.com>
@stuggi
Copy link
Contributor Author

stuggi commented Dec 21, 2024

/cherry-pick 18.0-fr1

@openshift-cherrypick-robot

@stuggi: new pull request created: #1244

In response to this:

/cherry-pick 18.0-fr1

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants