chef-cli icon indicating copy to clipboard operation
chef-cli copied to clipboard

New Policyfile.lock.json generated during `chef push`

Open emachnic opened this issue 9 years ago • 29 comments

Version:

ChefDK 0.13.21

Environment:

This happens on both my local Mac computer and remote Windows CI server.

Scenario:

I wasn't sure if this was the desired behavior so I wanted to create an issue for clarification. Basically, I'm trying to have my CI server run Chef tests and then push the policy after everything has passed. Unfortunately, the current behavior of regenerating a Policyfile.lock.json when running chef push means that the latest lock file in the remote repo will not be the one that is on the Chef server.

Steps to Reproduce:

  1. Using a cookbook with a Policyfile, run chef update to update the Policy revision.
  2. Commit files (make note of the revision in the Policyfile.lock.json) and push to remote repository
  3. Run chef push <policy_group> to push policy to the group

Expected Result:

I expect the policy revision that was created with the chef update command to be the one that is on the Chef server in the policy group and in the remote repository.

Actual Result:

  • A new policy revision is generated during the chef push command.
  • The Git repo is dirty because the Policyfile.lock.json has been modified and not committed
  • The policy revision on the Chef server is the new one from the uncommitted Policyfile.lock.json

I think this has to do with the call to write_updated_lockfile in the Push class. It seems to me that the lockfile should be validated and then pushed without writing another one.

Corresponding line in code: https://github.com/chef/chef-dk/blob/75ecb686077ba901896f0b77194da94c2830cf2e/lib/chef-dk/policyfile_services/push.rb#L78

emachnic avatar May 11 '16 16:05 emachnic

Is there any update on this? It's definitely affecting our Chef cookbook workflow mojo.

Just found out that running Chefspec with Policyfiles also causes a new Policyfile.lock.json to be created. So that really screws with using Policyfiles in a CI environment.

emachnic avatar May 17 '16 16:05 emachnic

Just figured out that this also means there is no way for all policy groups to have the same revision. That's probably the biggest issue.

emachnic avatar May 17 '16 18:05 emachnic

Can someone comment that this has at least been seen?

Cheers!

emachnic avatar Jun 02 '16 13:06 emachnic

I have seen it. Have not had a chance to dig in but plan to unless someone else beats me to it.

mwrock avatar Jun 02 '16 13:06 mwrock

Thank @mwrock. Let me know if you need more infoz

emachnic avatar Jun 02 '16 14:06 emachnic

@mwrock I was trying to go through the code to see if I could potentially submit a patch. What I'm unclear on is if there's a case where write_updated_lockfile actually needs to be called within the push command. It seems to me that push should only be concerned with seeing if there's a valid lock file and validating it then pushing the lock file that's there. The push spec says "validates the lockfile, writes any updates, and uploads the cookbooks" but I don't see a use case for "writes any updates". Am I completely off base?

emachnic avatar Jun 02 '16 18:06 emachnic

I took a close look at this just now (sorry for the delay). I have to admit that I agree with you that this does not seem right. It feels like this easily enables scenarios where the principle of least surprise could be very much violated.

mwrock avatar Jun 03 '16 23:06 mwrock

I'm facing the same issue. I also thought I'd be able to push the .lock.json file. But chef push expects an .rb and regens .lock.json, which may be different.

laureled avatar Jun 26 '16 15:06 laureled

We have found that we should never use chef update. We've found it to be unreliable.

So in your CI when you're regenerating a lockfile, run

rm Policyfile.lock.json
chef install Policyfile.rb

And then check that in.

Then in CI when you're pushing it, you need to make sure that the cookbooks are cached locally, and you do this with chef install:

chef install Policyfile.rb
chef push qa Policyfille.rb

In short, don't use chef update with your workflow

mhedgpeth avatar Nov 03 '16 17:11 mhedgpeth

@mhedgpeth:

We have found that we should never use chef update. We've found it to be unreliable.

Can you please clarify what you mean by "unreliable"? Perhaps with examples? This issue seems to be mostly concerned with chef push, which unexpectedly triggers an update to the policyfile, so I'm curious what problem you're having and how it relates to this issue.

jayhendren avatar Nov 03 '16 17:11 jayhendren

I'm providing the workaround/solution to the issue.

Problem was when running chef update and then chef push, it is providing a different version of a json. This is because the chef update is always updating the json file. This is what I mean by unreliable; from a user perspective it's doing different things than you would expect.

I expect chef update to not update the json file but it does.

The workaround is to do as I say above: delete the json file & run chef install if you want to create it. Otherwise run chef install and it will do what you thought chef update did before.

So if this bug remains open, I believe it's really due to chef update doing things that people don't expect.

mhedgpeth avatar Nov 03 '16 17:11 mhedgpeth

chef push updates the file too.

theckman avatar Nov 03 '16 17:11 theckman

I want to lock my dependencies and never have the lock file change unless I explicitly run chef update. We should make it so chef push can fail if there are changes.

theckman avatar Nov 03 '16 17:11 theckman

I just successfully followed the following workflow on this version: Chef Development Kit Version: 0.16.28

rm Policyfile.lock.json
chef install Policyfile.rb
# note the revision id here
chef push development Policyfile.rb
chef show-policy policyname

It outputs the revision id that was generated after the chef install. This workflow works.

mhedgpeth avatar Nov 03 '16 17:11 mhedgpeth

@mhedgpath what does git status show after the Chef push? Can you also try running RSpec with a clean working directory and then see if it's still clean? The issue is that these tasks should not regenerate the lock file and cause the working copy to be dirty.

Evan On Thu, Nov 3, 2016 at 12:42 PM mhedgpeth [email protected] wrote:

I just successfully followed the following workflow on this version: Chef Development Kit Version: 0.16.28

rm Policyfile.lock.json chef install Policyfile.rb

note the revision id here

chef push development Policyfile.rb chef show-policy policyname

It outputs the revision id that was generated after the chef install. This workflow works.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/chef/chef-dk/issues/831#issuecomment-258218605, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKUOgUbtv-B4CvLHHcrEy0VQK8SNByuks5q6hzsgaJpZM4IcRym .

emachnic avatar Nov 03 '16 17:11 emachnic

@mhedgpeth if you commit all your files between the install and the push, tell me if your lockfile has a diff after the push.

theckman avatar Nov 03 '16 17:11 theckman

@mhedgpeth:

I expect chef update to not update the json file but it does.

I think you misunderstand what chef update is designed to do. From the help output:

[birdsnest ~]% chef update --help
Usage: chef update [ POLICY_FILE ] [options]

`chef update` reads your `Policyfile.rb`, applies any changes, re-solves the
dependencies and emits an updated `Policyfile.lock.json`. The new locked policy
will reflect any changes to the `run_list` and pull in any cookbook updates
that are compatible with the version constraints stated in your `Policyfile.rb`.

[...]

Contradictory to your expectations, chef update is explicitly designed to update the json lockfile.

jayhendren avatar Nov 03 '16 17:11 jayhendren

My bad, I think I got confused with the command there. In a discussion on slack it looks like this problem is due to there being changes exist in the cookbook that aren't a part of the chefignore file. So the key to making chef push to not regenerate a new policy is to ensure that the contents of the cookbook (especially if there is a cookbook reference to .) haven't changed, and if they did, that they're a part of the chefignore. I hope that makes sense.

mhedgpeth avatar Nov 03 '16 21:11 mhedgpeth

@mhedgpeth Thank you for your suggestion. Unfortunately, in my case the file is updating because the SCM info is changing. Specifically, the git commit of the repo the lockfile is in has changed, not because anything within the cookbook has changed. I'm not sure how useful of a feature that is for the chef push semantics.

If you have any suggestions around avoiding the SCM update I'm super interested.

theckman avatar Nov 04 '16 02:11 theckman

Closing because this seems to be working as documented.

cheeseplus avatar Jul 12 '17 21:07 cheeseplus

@cheeseplus I'm not sure that's the case... The original issue was about chef push causing an unexpected rewrite of Policyfile.lock.json. The comments then got sidetracked into a few different issues, but as far as I can tell, the OP still has not been addressed.

jayhendren avatar Jul 12 '17 21:07 jayhendren

I agree with @Poohblah. I think this issue may need to remain open, because I don't think a chef push should update the lockfile. The lockfile should be locked, unless you run chef update.

theckman avatar Jul 12 '17 22:07 theckman

I have the same problem with this config. Chef Development Kit Version: 2.0.26 chef-client version: 13.2.20 delivery version: master (17c1b0fed9be4c70f69091a6d21a4cbf0df60a23) berks version: 6.2.0 kitchen version: 1.16.0 inspec version: 1.30.0 thanks @Poohblah We can't use policies anymore with it

abmptit avatar Aug 21 '17 13:08 abmptit

Any movement here? chef export is also updating the lock file. While this is more of an annoyance for us with our workflow, I can see this causing issues for others.

Commands like push and export really should not be writing the lock file at all (or at least they need a --no-update flag).

albertrdixon avatar Jun 27 '18 19:06 albertrdixon

I was poking at this earlier and it seems like the update is happening fairly deep in the flow. I'll take another look at this soon and see if I can't get a PR made to at least make it optional, but no promises. Would like to add my 👍 to this being a... strange thing.

In our use case we're using CI to run chef export on our lockfiles and using chef-zero with the generated tarballs. Since the lockfile gets updated when generating, we can't use the committed lockfile to generate the tarball, which means there doesn't seem to be a whole lot of point to using a lockfile in the first place, at least for our case. I could be wrong, and would appreciate any feedback to the contrary :)

kitchen avatar Mar 21 '19 23:03 kitchen

Facing the same issue. Fixed by putting this following in the chefignore

# Policyfile #
##############
Policyfile.rb
Policyfile.lock.json

hchiao1 avatar Nov 07 '19 18:11 hchiao1

Unfortunately the chefignore fix did not work for me. chef push still generates changes to Policyfile.lock.json. I'm on ChefDK 3.13.1, not sure if a newer version would make a difference, but in my particular case I'm stuck on that version.

jayhendren avatar Jul 30 '20 21:07 jayhendren

This needs to be verified under the current Workstation build.

marcparadise avatar Dec 16 '20 18:12 marcparadise

I am investigating adopting Policyfiles and I am quite surprised to experience this behavior. Both chef push and chef export still update the lock file unexpectedly.

In my tests it happens if I edit a cookbook recipe and then run either of the command. I expect both commands to fail because chef is unable to satisfy the lock file (because of edited cookbook and lack of running chef update). The way it works now means that I have no idea what actually got sent to Chef Server.

I thought the whole point of Policyfiles was a better predictability of the whole infrastructure, so I'm quite taken aback to see it fails this way...

@emachnic were you able to finally find a solution for your use-case ?

$ chef --version
Chef Workstation version: 22.7.1006
Chef Infra Client version: 17.10.0
Chef InSpec version: 4.56.20
Chef CLI version: 5.6.1
Chef Habitat version: 1.6.420
Test Kitchen version: 3.3.1
Cookstyle version: 7.32.1

PowerKiKi avatar Aug 24 '22 15:08 PowerKiKi