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

Upgrade crio to 1.15.0 #4703

Merged
merged 2 commits into from
Jul 16, 2019
Merged

Conversation

afbjorklund
Copy link
Collaborator

@afbjorklund afbjorklund commented Jul 7, 2019

The repository was moved to a separate organization

strings.ReplaceAll only work in go1.12, not go1.10

Need to create conmon/config.h without tags and git

The configuration needs to be updated (yet again)

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 7, 2019
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 7, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afbjorklund

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 7, 2019
@afbjorklund
Copy link
Collaborator Author

Currently doesn't build anymore, due to cri-o/cri-o#2450

runtime_vm.go:175:20: undefined: strings.ReplaceAll

@afbjorklund afbjorklund added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2019
@afbjorklund afbjorklund removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 7, 2019
@afbjorklund
Copy link
Collaborator Author

Reported the workarounds for conmon/config.h as cri-o/cri-o#2575

The repository was moved to a separate organization

strings.ReplaceAll only work in go1.12, not go1.10

Need to create conmon/config.h without tags and git

The configuration _needs_ to be updated (yet again)
@afbjorklund
Copy link
Collaborator Author

/retest

@medyagh
Copy link
Member

medyagh commented Jul 7, 2019

I could upgrade the go version on the jenkins nodes, would that be sufficient ? or do we need to update the travis config too?

@afbjorklund
Copy link
Collaborator Author

@medyagh : it is the buildroot go, and it will be upgraded when we move from 2018.x to 2019.x
But it was working ok in the previous cri-o (1.14) and it seemed like a petty reason to require it ?

# of trust of the workload.

[crio.runtime.runtimes.runc]
runtime_path = ""
Copy link
Member

Choose a reason for hiding this comment

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

Please be aware that runc has to be in $PATH to let this work during runtime.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, this is what the crio generated by default. Compare it with our config (above).

runtime_path = "/usr/bin/runc"

I generated and added the default config, since it changes and breaks all. the. time

  • plugin_dir, plugin_dirs

Copy link
Member

Choose a reason for hiding this comment

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

I guess it should work fine :)

${RELEASE_TOOL} -n $(release)

-conmon/config.h: git-vars cmd/crio-config/config.go oci/oci.go
+conmon/config.h: cmd/crio-config/config.go oci/oci.go
Copy link
Member

Choose a reason for hiding this comment

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

So the git executable is not available within your build environment? Because the git-vars target was introduced to actually fix this, but it only checks for the .git folder. I think I can improve this :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually it was worse... We do have git, but that's for our buildroot. Thus it recorded commits from a different project. We build all our packages from tarballs, so have to supply the matching commit for each one.

The main problem here is that the "git-vars" target forces the conmon/config.h to be re-generated, even if it already exists (same with crio.conf too)

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see, thanks. Let's see if we can get rid of the git-vars dependency of this target entirely.

@afbjorklund
Copy link
Collaborator Author

On the plus side, the new release seem to be working fine with the tweaks made.

Haven't tried it with the new runc yet, need to make a trial run with all the changes.

@afbjorklund
Copy link
Collaborator Author

/retest

@afbjorklund
Copy link
Collaborator Author

/retest

@afbjorklund
Copy link
Collaborator Author

/test Jenkins Cross Build

@tstromberg
Copy link
Contributor

@minikube-bot OK to test

@afbjorklund afbjorklund merged commit 35753aa into kubernetes:master Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants