Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

ci: Enable check VERSION among the components without the runtime#614

Merged
jcvenegas merged 1 commit intokata-containers:masterfrom
GabyCT:topic/updatecheckversion
Jul 5, 2019
Merged

ci: Enable check VERSION among the components without the runtime#614
jcvenegas merged 1 commit intokata-containers:masterfrom
GabyCT:topic/updatecheckversion

Conversation

@GabyCT
Copy link
Contributor

@GabyCT GabyCT commented Jul 3, 2019

The main purpose is that this script will be used to verify
that VERSION among the components are equal before merging the runtime.

Fixes #613

Depends-on: github.com/kata-containers/runtime#1858

Signed-off-by: Gabriela Cervantes gabriela.cervantes.tellez@intel.com

check_versions
;;
pre-release)
unset repos[2]
Copy link
Member

@jcvenegas jcvenegas Jul 3, 2019

Choose a reason for hiding this comment

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

Instead of do this could you try.

check_versions $(cat "${GOPATH}/src/${runtime_repo}/VERSION")
...
#$1 Version to check to branches merged
check_versions() {
version_to_check=${1:-}
if version_to_check == "";then
info query the version from latest runtime in branch ${branch}
else
    kata_version=${version_to_check}
fi
...
for repo in "${repos[@]}"; do
  if repo == kata-runtime; then
      not checking runtime because we want the rest of repos are in $version_to_check
      continue
  fi
}

Copy link
Contributor

Choose a reason for hiding this comment

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

yep - let's do a specific check or some smart way to 'build up' the list of repos to check, and not add the runtime to the list in the first place if we are doing the pre-check.

#The runtime version is used as reference of latest release
# This is set to the right value later.
kata_version=""
GOPATH="${HOME}/go"
Copy link
Contributor

Choose a reason for hiding this comment

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

':-' idiom to set only if not set maybe?

check_versions
;;
pre-release)
unset repos[2]
Copy link
Contributor

Choose a reason for hiding this comment

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

yep - let's do a specific check or some smart way to 'build up' the list of repos to check, and not add the runtime to the list in the first place if we are doing the pre-check.

pre-release)
unset repos[2]
runtime_repo="github.com/kata-containers/runtime"
export VERSION_TO_CHECK=$(cat "${GOPATH}/src/${runtime_repo}/VERSION") && check_versions
Copy link
Contributor

Choose a reason for hiding this comment

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

I think/guess this is reply on the runtime repo having been already checked out to the correct PR - as this script will be called for by the runtime repo code on a runtime repo PR being checked.... maybe a small comment to note that - otherwise it could be a little confusing :-)

The main purpose is that this script will be used to verify
that VERSION among the components are equal before merging the runtime.

Fixes kata-containers#613

Depends-on: github.com/kata-containers/runtime#1858

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
@GabyCT GabyCT force-pushed the topic/updatecheckversion branch 3 times, most recently from 15e01be to 589e1a9 Compare July 4, 2019 16:27
@GabyCT
Copy link
Contributor Author

GabyCT commented Jul 4, 2019

@jcvenegas changes applied

GabyCT added a commit to GabyCT/runtime-1 that referenced this pull request Jul 4, 2019
This will take the VERSION of all the components in order to
verify that they match among them before merging the runtime.

Fixes kata-containers#1581

Depends-on: github.com/kata-containers/packaging#614

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
Copy link
Member

@jcvenegas jcvenegas left a comment

Choose a reason for hiding this comment

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

Thanks gaby a few additional comments.

;;
pre-release)
runtime_repo="github.com/kata-containers/runtime"
check_versions $(cat "${GOPATH}/src/${runtime_repo}/VERSION")
Copy link
Member

Choose a reason for hiding this comment

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

Almost there, I wonder if instead of assume that the runtime repo exist here, could you read the version as a second parameter?
so in travis you can do something like
tag_repos.sh pre-release $(cat VERSION)
this also helps to remove the GOPATH variable and runtime_repo

  • Also could you add some usage info ?
  • finally you can add some basic test here tag_repos_test.sh
    tag_repos.sh pre-release 100000 will fail
    or
    tag_repos.sh pre-release $(curl https://github.com/kata-containers/runtime/blob/master/VERSION) I think should work

@GabyCT GabyCT force-pushed the topic/updatecheckversion branch 2 times, most recently from d263555 to 98ad9e2 Compare July 4, 2019 18:40
GabyCT added a commit to GabyCT/runtime-1 that referenced this pull request Jul 4, 2019
This will take the VERSION of all the components in order to
verify that they match among them before merging the runtime.

Fixes kata-containers#1581

Depends-on: github.com/kata-containers/packaging#614

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
@GabyCT
Copy link
Contributor Author

GabyCT commented Jul 4, 2019

@jcvenegas changes applied

@jcvenegas
Copy link
Member

/test

@jcvenegas
Copy link
Member

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

GabyCT added a commit to GabyCT/runtime-1 that referenced this pull request Jul 5, 2019
This will take the VERSION of all the components in order to
verify that they match among them before merging the runtime.

Fixes kata-containers#1581

Depends-on: github.com/kata-containers/packaging#614

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
@jcvenegas jcvenegas merged commit 3240ad0 into kata-containers:master Jul 5, 2019
GabyCT added a commit to GabyCT/runtime-1 that referenced this pull request Jul 5, 2019
This will take the VERSION of all the components in order to
verify that they match among them before merging the runtime.

Fixes kata-containers#1581

Depends-on: github.com/kata-containers/packaging#614

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
GabyCT added a commit to GabyCT/runtime-1 that referenced this pull request Jul 5, 2019
This will take the VERSION of all the components in order to
verify that they match among them before merging the runtime.

Fixes kata-containers#1581

Depends-on: github.com/kata-containers/packaging#614

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
jcvenegas pushed a commit to jcvenegas/runtime that referenced this pull request Oct 8, 2019
This will take the VERSION of all the components in order to
verify that they match among them before merging the runtime.

Fixes kata-containers#1581

Depends-on: github.com/kata-containers/packaging#614

Signed-off-by: Gabriela Cervantes <gabriela.cervantes.tellez@intel.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable check VERSION among the components without the runtime

4 participants