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

Edit Deployment Files by Kubernetes v1.16.13 and Fluidcr Registry #3848

Merged
merged 5 commits into from
Oct 9, 2020

Conversation

tianzhu007
Copy link
Contributor

@tianzhu007 tianzhu007 commented Oct 6, 2020

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

@tianzhu007 tianzhu007 changed the title Edit kafka and nginx yaml by updating the k8s version Edit Kafka and Nginx Deployment yaml from 1.16.13 k8s version Oct 6, 2020
@tianzhu007 tianzhu007 changed the title Edit Kafka and Nginx Deployment yaml from 1.16.13 k8s version Edit Kafka and Nginx Deployment Yaml from 1.16.13 K8s Version Oct 6, 2020
@tianzhu007 tianzhu007 changed the title Edit Kafka and Nginx Deployment Yaml from 1.16.13 K8s Version Edit Deployment Files by 1.16.13 K8s Version and Fluidcr Regsitry Oct 7, 2020
@tianzhu007 tianzhu007 changed the title Edit Deployment Files by 1.16.13 K8s Version and Fluidcr Regsitry Edit Deployment Files by 1.16.13 K8s Version and Fluidcr Registry Oct 7, 2020
@tianzhu007 tianzhu007 changed the title Edit Deployment Files by 1.16.13 K8s Version and Fluidcr Registry Edit Deployment Files by 1.16.13 Kubernetes Version and Fluidcr Registry Oct 7, 2020
@tianzhu007 tianzhu007 changed the title Edit Deployment Files by 1.16.13 Kubernetes Version and Fluidcr Registry Edit Deployment Files by Kubernetes v1.16.13 and Fluidcr Registry Oct 7, 2020
Comment on lines 6 to +7
name: historian
image: prague.azurecr.io/historian:4049
image: fluidcr.azurecr.io/build/fluidframework/routerlicious/historian:latest
Copy link
Contributor

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?

Copy link
Contributor Author

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

@tianzhu007 tianzhu007 Oct 7, 2020

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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. :)

@github-actions github-actions bot requested a review from tanviraumi October 7, 2020 22:32
@curtisman
Copy link
Member

Close and reopening to trigger the pipeline

@curtisman curtisman closed this Oct 8, 2020
@curtisman curtisman reopened this Oct 8, 2020
@@ -11,6 +11,9 @@ metadata:
spec:
serviceName: zookeeper
replicas: 1
selector:
matchLabels:
app: {{ template "kafka.fullname" . }}
Copy link
Contributor

Choose a reason for hiding this comment

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

zookeeper.fullname

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 this value be the same as template/metadata/labels/app's value, that is,
{{ template: "kafka.fullname" .}} ?

Copy link
Contributor

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.

Copy link
Contributor

@tanviraumi tanviraumi left a 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

@tanviraumi tanviraumi merged commit b7f095b into microsoft:main Oct 9, 2020
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.

3 participants