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

Remove all hardcoded EOL K8s version #1949

Merged
merged 1 commit into from
May 8, 2024

Conversation

collivier
Copy link
Collaborator

Description

cnf testsuite must be verified vs up-to-date K8S versions.
By removing the harcoded K8S versions, the gates will leverage the latest stable K8S version as proposed by Kind.

Issues:

Refs: #1948

How has this been tested:

A verification job must be executed to see the testcases which would have to be updated.
This PR may ask for others PR if obsolete test cases ask for updates.

Types of changes:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

@collivier
Copy link
Collaborator Author

collivier commented Apr 5, 2024

@agentpoyo @HashNuke could you please have a look to the failures as for instance
https://github.com/cnti-testcatalog/testsuite/actions/runs/8567585939/job/23479668576?pr=1949

They all failed because of Error: Cannot perform an interactive login from a non TTY device which makes me think that DOCKERHUB_USERNAMES and DOCKERHUB_PASSWORDS are empty.

This error can be easily reproduce by calling docker login -u -p from a non tty device

@HashNuke
Copy link
Collaborator

HashNuke commented Apr 9, 2024

I just pushed a build to GitHub Actions. I have a wild guess what might be causing this issue. I'll share some notes incase the build fails.

@HashNuke
Copy link
Collaborator

HashNuke commented Apr 9, 2024

There are some failing tests. Here's the build - https://github.com/cnti-testcatalog/testsuite/actions/runs/8611760709
I've tried re-running the failed tests again. They should complete soon.

Edit: Made a mistake and pushed the branch again. So I created a new build. I'll let it complete for reference - https://github.com/cnti-testcatalog/testsuite/actions/runs/8614014853)

@collivier I checked the changes in this PR. Looks like this is related to an older issue.

Here is some info I dug up from my notes:

  • New kind clusters for k8s 1.25+ are based on the Distroless project.
  • These are minimal. They do not contain tar, sh and few other things.

For GitHub Actions to work we'll have to configure to the kindest/node image that is the latest version not based on distroless. This is the reason that the kind node image was hardcoded in the GitHub Actions.

I posted some notes in an older issue:
#1631 (comment)

From what I can remember, some parts of the testsuite rely on being able to ssh into the node and run certain commands (cluster-tools most likely). The distroless images don't help with this. Fixing the root cause would mean looking into this specifically.

@collivier
Copy link
Collaborator Author

@HashNuke I frankly don't get your points, sorry.

Many issues can easily fix by removing the useless -ti in kubectl calls.

$ git diff
diff --git a/spec/airgap_task_spec.cr b/spec/airgap_task_spec.cr
index 3f6c724e..c14f3cc5 100644
--- a/spec/airgap_task_spec.cr
+++ b/spec/airgap_task_spec.cr
@@ -44,9 +44,9 @@ describe "AirGap" do
     # Get the generated name of the cri-tools per node
     pods.map do |pod| 
       pod_name = pod.dig?("metadata", "name")
-      sh = KubectlClient.exec("-ti #{pod_name} -- cat /usr/local/bin/crictl > /dev/null")  
+      sh = KubectlClient.exec("#{pod_name} -- cat /usr/local/bin/crictl > /dev/null")
       sh[:status].success?
-      sh = KubectlClient.exec("-ti #{pod_name} -- cat /usr/local/bin/ctr > /dev/null")  
+      sh = KubectlClient.exec("#{pod_name} -- cat /usr/local/bin/ctr > /dev/null")
       sh[:status].success?
     end
     (/All prerequisites found./ =~ response_s).should_not be_nil

You're free to install any software in the kind images

$ sudo docker exec -ti kind-control-plane /bin/bash
root@kind-control-plane:/# apt-get update
Get:1 http://deb.debian.org/debian bookworm InRelease [151 kB]
Get:2 http://deb.debian.org/debian bookworm-updates InRelease [55.4 kB]
Get:3 http://deb.debian.org/debian-security bookworm-security InRelease [48.0 kB]
Get:4 http://deb.debian.org/debian bookworm/main amd64 Packages [8786 kB]
Get:5 http://deb.debian.org/debian bookworm-updates/main amd64 Packages [12.7 kB]                                                                                                                                 
Get:6 http://deb.debian.org/debian-security bookworm-security/main amd64 Packages [151 kB]                                                                                                                        
Fetched 9204 kB in 7s (1318 kB/s)                                                                                                                                                                                 
Reading package lists... Done
root@kind-control-plane:/# apt-get install tar
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
tar is already the newest version (1.34+dfsg-1.2+deb12u1).
tar set to manually installed.
0 upgraded, 0 newly installed, 0 to remove and 9 not upgraded.
root@kind-control-plane:/# 

I'm diving into the litmus-chaos based tests in parallel which looks more difficult

@agentpoyo agentpoyo added cnti-certification Enhancements to support CNTI Certification program v1.0.0 Issue included in v1.0.0 release and removed v1.0.0 Issue included in v1.0.0 release labels Apr 16, 2024
collivier added a commit to collivier/testsuite that referenced this pull request Apr 17, 2024
It avoids passing stdin to the container when useless.
Please note it removes a comple of issues higlighted by [1] in
airgap specs.

It also removes all useless blank chars in the files modified
by this change.

[1] cnti-testcatalog#1949

Signed-off-by: Cédric Ollivier <[email protected]>
collivier added a commit to collivier/testsuite that referenced this pull request Apr 17, 2024
It avoids passing stdin to the container when useless.
Please note it removes a couple of issues highlighted by [1] in
airgap specs.

It also removes all useless blank chars in the files modified
by this change.

[1] cnti-testcatalog#1949

Signed-off-by: Cédric Ollivier <[email protected]>
@agentpoyo
Copy link
Collaborator

I believe this PR is now outdated by #1998 which was merged already. @wavell @denverwilliams can you confirm?

haskojur pushed a commit to haskojur/testsuite that referenced this pull request Apr 24, 2024
It avoids passing stdin to the container when useless.
Please note it removes a couple of issues highlighted by [1] in
airgap specs.

It also removes all useless blank chars in the files modified
by this change.

[1] cnti-testcatalog#1949

Signed-off-by: Cédric Ollivier <[email protected]>
@collivier
Copy link
Collaborator Author

@agentpoyo @wavell @denverwilliams I locally rebased this PR on top of main. It remains 3 changes removing the hardcoded obsolete K8S versions. Do you want me to pin v1.29.2 as you do in the other PR or do I just push the rebase here ?

@wvwatson
Copy link
Collaborator

I'm thinking @denverwilliams is saying that it should stay pinned to avoid intermittent failure. Maybe we should update every month or so?

@collivier
Copy link
Collaborator Author

I'm thinking @denverwilliams is saying that it should stay pinned to avoid intermittent failure. Maybe we should update every month or so?

@denverwilliams @wavell It's indirectly pinned via the kind version. I can doubly hardcode the version if really needed. please let me know

@denverwilliams
Copy link
Collaborator

@collivier Pinning via the Kind Version at the top level and removing the image pinning is fine.

The only reason we were pinning the kind image was because we were installing Kind Latest.

@denverwilliams denverwilliams merged commit 47958a0 into cnti-testcatalog:main May 8, 2024
14 of 94 checks passed
@taylor
Copy link
Collaborator

taylor commented May 9, 2024

Note:

The test spec test for pod_memory_hog is failing https://github.com/cnti-testcatalog/testsuite/actions/runs/9020615104/job/24793752233 after this was merged. It seems to be a problem with the spec for that test and not the test itself. It also does not appear to be related to the changes in this PR.

cc: @denverwilliams

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cnti-certification Enhancements to support CNTI Certification program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants