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

Move daemonset API to apps/v1 #3660

Merged
merged 1 commit into from
Aug 29, 2019
Merged

Conversation

Pensu
Copy link
Contributor

@Pensu Pensu commented Jul 14, 2019

Fixes #3658

@murali-reddy
Copy link
Contributor

@Pensu thanks for the PR. weave-daemonset.yaml is for older version of Kubernetes. I opened #3674 to avoid confusion.

Since apps/v1 is GA in 1.9 and enabled by default I think it would be appropriate to introduce weave-daemonset-k8s-1.9.yaml. Would you mind updating the PR on the similar lines to #3353?

At some point it make sense to delete weave-daemonset-k8s-1.6/1.7/1.8.yaml which are manifests for very old versions.

@Pensu
Copy link
Contributor Author

Pensu commented Aug 1, 2019

@murali-reddy Yeah, sure, I have updated the PR. Please check and let me know if it's ok.

@bboreham
Copy link
Contributor

bboreham commented Aug 1, 2019

I think you need to start from the 1.8 version not from the pre-1.6 version.

@Pensu
Copy link
Contributor Author

Pensu commented Aug 6, 2019

I think you need to start from the 1.8 version not from the pre-1.6 version.

@bboreham Sorry, didn't get it. I just followed the changes from #3353

@bboreham
Copy link
Contributor

bboreham commented Aug 6, 2019

It looks like your new file weave-daemonset-k8s-1.9.yaml is based off https://github.com/weaveworks/weave/blob/master/prog/weave-kube/weave-daemonset.yaml.

It needs to be based off https://github.com/weaveworks/weave/blob/master/prog/weave-kube/weave-daemonset-k8s-1.8.yaml

For instance, that last file is 208 lines long; I'd expect yours to be about the same.

@Pensu
Copy link
Contributor Author

Pensu commented Aug 12, 2019

@bboreham Yeah, I have updated the PR by basing the 1.9 yaml off 1.8 one.

@murali-reddy
Copy link
Contributor

@Pensu Looks like you missed change to update apiVersion to apps/v1 in weave-daemonset-k8s-1.9.yaml

@Pensu
Copy link
Contributor Author

Pensu commented Aug 27, 2019

@murali-reddy Oh, yeah, thanks for pointing it out. Have updated the PR.

@murali-reddy
Copy link
Contributor

LGTM

@bboreham
Copy link
Contributor

I've kicked off another CI build.
Note we should include the change made in #3655 in this new file too.

To show the changes:

$ diff -c prog/weave-kube/weave-daemonset-k8s-1.8.yaml prog/weave-kube/weave-daemonset-k8s-1.9.yaml
*** prog/weave-kube/weave-daemonset-k8s-1.8.yaml	2018-11-01 14:16:48.557796495 +0000
--- prog/weave-kube/weave-daemonset-k8s-1.9.yaml	2019-08-28 10:06:08.153119861 +0000
***************
*** 100,106 ****
        - kind: ServiceAccount
          name: weave-net
          namespace: kube-system
!   - apiVersion: extensions/v1beta1
      kind: DaemonSet
      metadata:
        name: weave-net
--- 100,106 ----
        - kind: ServiceAccount
          name: weave-net
          namespace: kube-system
!   - apiVersion: apps/v1
      kind: DaemonSet
      metadata:
        name: weave-net
***************
*** 109,114 ****
--- 109,117 ----
        namespace: kube-system
      spec:
        # Wait 5 seconds to let pod connect before rolling next pod
+       selector:
+         matchLabels:
+           name: weave-net
        minReadySeconds: 5
        template:
          metadata:

@Pensu
Copy link
Contributor Author

Pensu commented Aug 29, 2019

@bboreham I have added the changes from #3655. Please check.

@bboreham
Copy link
Contributor

CI succeeded at https://circleci.com/gh/weaveworks/weave/12330 - thanks!

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.

Update the API for DaemonSet
3 participants