-
Notifications
You must be signed in to change notification settings - Fork 535
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
Edit Deployment Files by Kubernetes v1.16.13 and Fluidcr Registry #3848
Edit Deployment Files by Kubernetes v1.16.13 and Fluidcr Registry #3848
Conversation
name: historian | ||
image: prague.azurecr.io/historian:4049 | ||
image: fluidcr.azurecr.io/build/fluidframework/routerlicious/historian:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't you overriding these values in your deployment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. I have overridden these values when doing the deployment and update these changes here. :)
@@ -52,7 +52,7 @@ spec: | |||
options: | |||
- name: single-request-reopen | |||
imagePullSecrets: | |||
- name: regsecret | |||
- name: fluidcrregsecret |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same logic as another file.
@@ -42,7 +42,7 @@ spec: | |||
options: | |||
- name: single-request-reopen | |||
imagePullSecrets: | |||
- name: regsecret | |||
- name: fluidcrregsecret |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a new secret called fluidcrregsecret for the fluidcr.
The old regsecret remains there and keeps the same definition to pull images from prague.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to flip it. Update regsecret to point to new fluid registry and then create a new secret "legacyRegSecret" that points to Prague. Then you don't need to change these files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise, people using these deployment files will need to update their cluster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Changed it back. :)
Close and reopening to trigger the pipeline |
@@ -11,6 +11,9 @@ metadata: | |||
spec: | |||
serviceName: zookeeper | |||
replicas: 1 | |||
selector: | |||
matchLabels: | |||
app: {{ template "kafka.fullname" . }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zookeeper.fullname
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this value be the same as template/metadata/labels/app's value, that is,
{{ template: "kafka.fullname" .}} ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah you are correct. I forgot that we use the same name for Kafka and zookeeper. Makes sense. I will merge it now. Thanks for the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will merge when the zookeeper.name thing is fixed
Reference: https://kubernetes.io/blog/2019/07/18/api-deprecations-in-1-16/
StatefulSet in the apps/v1beta1 and apps/v1beta2 API versions is no longer served.
Migrate to use the apps/v1 API version, available since v1.9. Existing persisted data can be retrieved/updated via the new version.
Deployment in the extensions/v1beta1, apps/v1beta1, and apps/v1beta2 API versions is no longer served
Migrate to use the apps/v1 API version, available since v1.9. Existing persisted data can be retrieved/updated via the new version.
Edit historian deployment files to use the latest image from fluidcr registry