NETOBSERV-2019: Make sure the required yq version is installed #140

msherif1234 · 2024-12-16T21:35:34Z

Description

make sure the required YQ is available for the cli scripts to work

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

openshift-ci-robot · 2024-12-16T21:35:38Z

msherif1234 · 2024-12-16T22:18:00Z

/ok-to-test

github-actions · 2024-12-16T22:19:45Z

New image:
quay.io/netobserv/network-observability-cli:24e73b9

It will expire after two weeks.

To use this build, update your commands using:

USER=netobserv VERSION=24e73b9 make commands

or download the updated commands.

codecov · 2024-12-16T22:24:19Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 22.66%. Comparing base (d541d19) to head (c32649e).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #140   +/-   ##
=======================================
  Coverage   22.66%   22.66%           
=======================================
  Files          10       10           
  Lines        1337     1337           
=======================================
  Hits          303      303           
  Misses       1015     1015           
  Partials       19       19

Flag	Coverage Δ
unittests	`22.66% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

msherif1234 · 2024-12-16T22:31:19Z

to test this running locally
1- I install older version of yq
GOFLAGS="" go install github.com/mikefarah/yq/[email protected]
2- run the binary build by this PR
./test_commands packets --log-level=trace --protocol=TCP --port=6443

Installing yq version v4.43.1 is required. Found v4.40.3.
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100 9716k  100 9716k    0     0  13.7M      0 --:--:-- --:--:-- --:--:-- 23.1M
Successfully downloaded yq version v4.43.1.
Setting up... 
Error from server (NotFound): the server could not find the requested resource (get users.user.openshift.io ~)
creating netobserv-cli namespace
namespace/netobserv-cli created
creating service account
serviceaccount/netobserv-cli created
clusterrole.rbac.authorization.k8s.io/netobserv-cli unchanged
clusterrolebinding.rbac.authorization.k8s.io/netobserv-cli unchanged
creating collector service
service/collector created
creating packet-capture agents
opt: log_level, evalue: trace
opt: filter_protocol, evalue: TCP
opt: filter_port, evalue: 6443
daemonset.apps/netobserv-cli created
Waiting for daemon set "netobserv-cli" rollout to finish: 0 of 2 updated pods are available...
^CRunning network-observability-cli get-packets... 
pod/collector created
pod/collector condition met
Starting network-observability-cli:
=====
Build version: main-24e73b9
Build date: 2024-12-16 22:18

commands/netobserv

scripts/functions.sh

msherif1234 · 2024-12-17T14:09:05Z

/ok-to-test

github-actions · 2024-12-17T14:11:06Z

New image:
quay.io/netobserv/network-observability-cli:c18479c

It will expire after two weeks.

To use this build, update your commands using:

USER=netobserv VERSION=c18479c make commands

or download the updated commands.

jpinsonneau · 2024-12-17T15:17:48Z

scripts/dependencies_check.sh

@@ -0,0 +1,65 @@
+#!/usr/bin/env bash
+
+set -e


Suggested change

set -e

jpinsonneau · 2024-12-17T15:19:41Z

scripts/dependencies_check.sh

+  YQ_BIN=$(which yq 2>/dev/null)
+  if [ -z "$YQ_BIN" ]; then
+    echo "Error: 'yq' is not installed or not in PATH."
+    install_yq


Suggested change

install_yq

install_yq "$2"

jpinsonneau · 2024-12-17T15:22:05Z

scripts/dependencies_check.sh

+  for arch in $supported_archs; do
+    echo "Attempting to download yq version $required_yq_version for $OS/$arch..."
+    DOWNLOAD_URL="https://github.com/mikefarah/yq/releases/download/$required_yq_version/yq_${OS}_${arch}"
+    if ! curl -Lo "$YQ_BIN" "$DOWNLOAD_URL" && chmod +x "$YQ_BIN"


Suggested change

if ! curl -Lo "$YQ_BIN" "$DOWNLOAD_URL" && chmod +x "$YQ_BIN"

if curl -Lo "$YQ_BIN" "$DOWNLOAD_URL" && chmod +x "$YQ_BIN"

ugh was trying to fix linter and I broke it :(

jpinsonneau · 2024-12-17T15:23:56Z

scripts/dependencies_check.sh

+function install_yq() {
+  OS=$(uname | tr '[:upper:]' '[:lower:]') # Get the OS type (linux or darwin)
+  supported_archs="$1"
+  YQ_BIN="./yq"


That's a bit dirty 🤔

Is there a way to put it in /tmp folder for all envs ?
Or maybe alongside oc-netobserv command but renamming yq to something else

Also, each time the script runs, it redownload the yq binary. We need to check if that path exists at the top of this script and use it if available prior to the one returned by which yq

I put it in /tmp

Signed-off-by: Mohamed Mahmoud <[email protected]>

openshift-ci-robot · 2024-12-18T13:17:02Z

jpinsonneau · 2024-12-18T13:28:01Z

Took the opportunity to improve the display e2e41ca

Everything is working fine 👌 thanks @msherif1234 !

msherif1234 · 2024-12-18T13:42:41Z

@memodi will give this a try ?
/ok-to-test

github-actions · 2024-12-18T13:44:49Z

New image:
quay.io/netobserv/network-observability-cli:e7139e2

It will expire after two weeks.

To use this build, update your commands using:

USER=netobserv VERSION=e7139e2 make commands

or download the updated commands.

memodi · 2024-12-18T13:46:08Z

sure @msherif1234 , I upgraded my yq since , let me see if I can downgrade and give it a try, cc @Amoghrd you may want to try as well.

memodi

tested, it works! thanks @msherif1234

memodi · 2024-12-18T15:56:13Z

commands/netobserv

@@ -72,7 +72,11 @@ function packets() {
  esac
 }

-case "$1" in  
+required_yq_version="v0.0.0"


@msherif1234 - qq , where do you set the required version here?

its passed as an arg in the Makefile https://github.com/netobserv/network-observability-cli/pull/140/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R158

ah, I see it, the YQ_VERSION env var value already exists in main.

Amoghrd · 2024-12-18T16:03:26Z

/label qe-approved
Works as expected!

openshift-ci-robot · 2024-12-18T16:03:32Z

msherif1234 · 2024-12-18T17:00:13Z

/approve

openshift-ci · 2024-12-18T17:00:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msherif1234

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [msherif1234]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot added the jira/valid-reference label Dec 16, 2024

msherif1234 requested a review from jpinsonneau December 16, 2024 21:35

msherif1234 force-pushed the use_specific_yq branch 2 times, most recently from 41ed464 to d991299 Compare December 16, 2024 22:10

openshift-ci bot added the ok-to-test label Dec 16, 2024

jpinsonneau reviewed Dec 17, 2024

View reviewed changes

commands/netobserv Outdated Show resolved Hide resolved

jpinsonneau reviewed Dec 17, 2024

View reviewed changes

commands/netobserv Outdated Show resolved Hide resolved

jpinsonneau reviewed Dec 17, 2024

View reviewed changes

commands/netobserv Outdated Show resolved Hide resolved

jpinsonneau reviewed Dec 17, 2024

View reviewed changes

scripts/functions.sh Show resolved Hide resolved

jpinsonneau mentioned this pull request Dec 17, 2024

Misc-fixes #139

Merged

10 tasks

msherif1234 force-pushed the use_specific_yq branch from d991299 to f93beaa Compare December 17, 2024 13:30

github-actions bot removed the ok-to-test label Dec 17, 2024

msherif1234 requested a review from jpinsonneau December 17, 2024 13:31

msherif1234 force-pushed the use_specific_yq branch 2 times, most recently from 9f5f1ac to f4fcc21 Compare December 17, 2024 13:44

openshift-ci bot added the ok-to-test label Dec 17, 2024

jpinsonneau reviewed Dec 17, 2024

View reviewed changes

scripts/dependencies_check.sh Outdated

@@ -0,0 +1,65 @@

#!/usr/bin/env bash

set -e

Copy link

Contributor

jpinsonneau Dec 17, 2024

Choose a reason for hiding this comment

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

Suggested change

set -e

jpinsonneau reviewed Dec 17, 2024

View reviewed changes

Make sure the required yq version is installed

c32649e

Signed-off-by: Mohamed Mahmoud <[email protected]>

msherif1234 force-pushed the use_specific_yq branch from f4fcc21 to c32649e Compare December 17, 2024 17:17

github-actions bot removed the ok-to-test label Dec 17, 2024

msherif1234 requested a review from jpinsonneau December 17, 2024 18:02

polishing messages

e2e41ca

jpinsonneau approved these changes Dec 18, 2024

View reviewed changes

openshift-ci bot assigned jpinsonneau Dec 18, 2024

openshift-ci bot added the lgtm label Dec 18, 2024

openshift-ci bot added the ok-to-test label Dec 18, 2024

memodi approved these changes Dec 18, 2024

View reviewed changes

openshift-ci bot assigned memodi Dec 18, 2024

memodi reviewed Dec 18, 2024

View reviewed changes

openshift-ci bot added the qe-approved label Dec 18, 2024

openshift-ci bot added the approved label Dec 18, 2024

msherif1234 merged commit 082d6f3 into netobserv:main Dec 18, 2024
10 of 12 checks passed

	if ! curl -Lo "$YQ_BIN" "$DOWNLOAD_URL" && chmod +x "$YQ_BIN"
	if curl -Lo "$YQ_BIN" "$DOWNLOAD_URL" && chmod +x "$YQ_BIN"

NETOBSERV-2019: Make sure the required yq version is installed #140

NETOBSERV-2019: Make sure the required yq version is installed #140

Conversation

msherif1234 commented Dec 16, 2024 • edited by jpinsonneau Loading

Description

Dependencies

Checklist

openshift-ci-robot commented Dec 16, 2024 • edited by openshift-ci bot Loading

Description

Dependencies

Checklist

msherif1234 commented Dec 16, 2024

github-actions bot commented Dec 16, 2024

codecov bot commented Dec 16, 2024 • edited Loading

Codecov Report

msherif1234 commented Dec 16, 2024

msherif1234 commented Dec 17, 2024

github-actions bot commented Dec 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Dec 18, 2024 • edited by openshift-ci bot Loading

Description

Dependencies

Checklist

jpinsonneau commented Dec 18, 2024

msherif1234 commented Dec 18, 2024

github-actions bot commented Dec 18, 2024

memodi commented Dec 18, 2024

memodi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msherif1234 Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Amoghrd commented Dec 18, 2024

openshift-ci-robot commented Dec 18, 2024 • edited by openshift-ci bot Loading

Description

Dependencies

Checklist

msherif1234 commented Dec 18, 2024

openshift-ci bot commented Dec 18, 2024

msherif1234 commented Dec 16, 2024 •

edited by jpinsonneau

Loading

openshift-ci-robot commented Dec 16, 2024 •

edited by openshift-ci bot

Loading

codecov bot commented Dec 16, 2024 •

edited

Loading

openshift-ci-robot commented Dec 18, 2024 •

edited by openshift-ci bot

Loading

msherif1234 Dec 18, 2024 •

edited

Loading

openshift-ci-robot commented Dec 18, 2024 •

edited by openshift-ci bot

Loading