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

Kubernetes client version update #70

Merged
merged 2 commits into from
Aug 6, 2021

Conversation

srgothi92
Copy link
Contributor

@srgothi92 srgothi92 commented Aug 4, 2021

Issue number:
N/A

Description of changes:
This change consists of 2 commits:

  1. Kubernetes client version update
Kubernetes client version used in update operator was not updated
in long time and there were some new features added in latest version
that are very helpful in mindful draining of node. This, change updates
all the Kubernetes binaries version 0.21.3.
  1. Changes log level for posted intent to Info
Posted intent are very important logs which can help understand
the flow of update communication between controller and agent.
Therefore, changing it to level info so that we can easily navigate
important logs.

Testing done:

  1. Started a Kubernetes cluster with two instances and a nginx deployment with PDB and saw them getting updated successfully with brupop.

Nginx deployment yaml:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx-deployment
spec:
  selector:
    matchLabels:
      app: nginx
  replicas: 2 # tells deployment to run 2 pods matching the template
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - name: nginx
        image: nginx:1.14.2
        ports:
        - containerPort: 80
---
apiVersion: policy/v1beta1
kind: PodDisruptionBudget
metadata:
  name: zk-pdb
spec:
  minAvailable: 1
  selector:
    matchLabels:
      app: nginx

Details:

# before update
Name                                           OS
ip-1-2-3-4.us-west-2.compute.internal     Bottlerocket OS 1.0.6
ip-5-6-7-8.us-west-2.compute.internal     Bottlerocket OS 1.0.6
# After update
Name                                           OS
ip-1-2-3-4.us-west-2.compute.internal     Bottlerocket OS 1.1.4
ip-5-6-7-8.us-west-2.compute.internal    Bottlerocket OS 1.1.4

  1. Repeated step 1 testing for K8s 1.17, 1.18, 1.19, 1.20.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Kubernetes client version used in update operator was not updated
in long time and there were some new features added in latest version
that are very helpful in mindul draining of node. This, change updates
all the Kubernetes binaries version 0.21.3.
Posted intent are very imprortant logs which can help understand
the flow of update communication between controller and agent.
Therefore, changing it to level info so that we can easily navigate
important logs.
@srgothi92 srgothi92 changed the title ECS Version update Kubernetes client version update Aug 4, 2021
Copy link
Contributor

@samuelkarp samuelkarp left a comment

Choose a reason for hiding this comment

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

I'm not thrilled about context.TODO() everywhere, but we can merge this and address that later.

@srgothi92 Would you mind opening an issue to track the removal of context.TODO()?

@srgothi92
Copy link
Contributor Author

@srgothi92 Would you mind opening an issue to track the removal of context.TODO()?

I am not either, but i did not find a better way to set the context. Everywhere in Kubernetes client examples they are using context.TODO. Even reading about context document did not help. Let me know if you have some thought on how to approach the context.TODO() problem. In any case, I am creating the issue we can discuss details over there.

@srgothi92
Copy link
Contributor Author

I'm not thrilled about context.TODO() everywhere, but we can merge this and address that later.

@srgothi92 Would you mind opening an issue to track the removal of context.TODO()?

Done. #71

Copy link
Member

@gthao313 gthao313 left a comment

Choose a reason for hiding this comment

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

lgtm

@srgothi92 srgothi92 merged commit 4678425 into bottlerocket-os:develop Aug 6, 2021
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

3 participants