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

Set db-operator version on database CR during full reconcile #107

Merged
merged 6 commits into from
Apr 23, 2024

Conversation

bobertrublik
Copy link

The change is rather simple. I retrieve the pod db-operator runs in and extract the version from the image tag. I had to add a new POD_NAMESPACE environment variable to the db-operator deployment manifest so I can fetch the pod from the cluster.

@bobertrublik
Copy link
Author

I'm not sure if I should add more error handling when I split the image tag. What do you think?

@allanger
Copy link
Member

allanger commented Apr 7, 2024

I was thinking about something like that https://www.digitalocean.com/community/tutorials/using-ldflags-to-set-version-information-for-go-applications

It doesn't seem right to me that dbo will have to recieve data about itself from the k8s api. Fithermore, it doesn't have to be running in k8s

@bobertrublik
Copy link
Author

Ah I didn't know that possibility existed. Not an issue, I will implement this. 🙂

Do you mean it could also run on Openshift or what did you have in mind?

@allanger
Copy link
Member

allanger commented Apr 7, 2024

Do you mean it could also run on Openshift or what did you have in mind?

In general, it should be possible to run it as a bin pointing to k8s from outside

@bobertrublik
Copy link
Author

Does this go in the right direction?

@allanger
Copy link
Member

allanger commented Apr 8, 2024

Yep, it looks better to me, but I would put it to the status, not the spec

@bobertrublik bobertrublik force-pushed the 79-add-version branch 2 times, most recently from 5b13653 to bfd5cb7 Compare April 12, 2024 21:22
@bobertrublik
Copy link
Author

Fixed here and in the chart repository. If you want, you can test the image using this build target in the Makefile,.

build: ## Build a container
	$(CONTAINER_TOOL) build ${CONTAINER_TOOL_ARGS} -t ${IMAGE_TAG} . ${CONTAINER_TOOL_NAMESPACE_ARG} --build-arg="VERSION=v1.23.0"

Copy link
Member

@allanger allanger left a comment

Choose a reason for hiding this comment

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

I would replace version to operatorVersion
Also I'm not sure if the common helper is a good place for storing the var, but I can't find a better one, so I'm alright with that

api/v1beta1/database_types.go Outdated Show resolved Hide resolved
@bobertrublik
Copy link
Author

bobertrublik commented Apr 17, 2024

I renamed the variable but had issues running make generate because I'm on Go 1.22.1 which is incompatible with controller-gen 0.13.0. kubernetes-sigs/controller-tools#888

Are you fine with me updating the codebase and Github workflows to 1.22.1? After the controller-gen update the generated CRDs have a little different syntax for descriptions.

image

@allanger
Copy link
Member

Yep, why not. Let's upgrade

@bobertrublik bobertrublik force-pushed the 79-add-version branch 2 times, most recently from 8146289 to 2f93bd6 Compare April 21, 2024 21:39
@bobertrublik
Copy link
Author

I applied the necessary changes for the 1.22.1 update, renamed the variable to operatorVersion and copied the generated CRDs to the db-operator charts PR.

Copy link
Member

@allanger allanger left a comment

Choose a reason for hiding this comment

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

Thanks, it looks good to me now

@allanger allanger merged commit 75a4f4f into db-operator:main Apr 23, 2024
17 checks passed
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.

None yet

2 participants