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

Multiple errors when upgrading from v4.0.0 to v8.4.0 #650

Closed
brailsmt opened this issue Jan 28, 2021 · 7 comments
Closed

Multiple errors when upgrading from v4.0.0 to v8.4.0 #650

brailsmt opened this issue Jan 28, 2021 · 7 comments
Labels
Documentation Improvements or additions to documentation Waiting on Contributor Awaiting on the person who raised this to update

Comments

@brailsmt
Copy link

🗣️ Foreword

We are uplifting all our cookbooks to chef 16. As part of that effort, we have upgraded from the v4.0.0 version of the cookbook to the v8.4.0 version. When doing this uplift we encountered several issues described below.

👻 Brief Description

When uplifting to chef 16 from chef 14, we upgraded from v4.0.0 to v8.4.0 we experienced the following issues after uplifting nodes to chef 16 and re-running chef-client:

  • Cannot upgrade java version #649 The cookbook failed to install an upgraded version of java due to the symlink created with v4.0.0
  • /etc/profile.d/jdk.sh was not removed. This created an issue on oracle linux 7.9 where /etc/profile contains code to source every file in /etc/profile.d. Since java.sh sorts lexicographically before jdk.sh, this caused the java.sh to be sourced into the environment prior to jdk.sh, causing JAVA_HOME to be set to the old version JAVA_HOME location, ie. /usr/lib/jvm/java-8-adoptopenjdk-hotspot instead of /usr/lib/jvm/java-8-adoptopenjdk-hotspot/jdk8u265-b01. The relevant snippet from /etc/profile is:
for i in /etc/profile.d/*.sh ; do
    if [ -r "$i" ]; then
        if [ "${-#*i}" != "$-" ]; then
            . "$i"
        else
            . "$i" >/dev/null 2>&1
        fi
    fi
done
  • the jdk-version-changed group is not notified when the jdk is upgraded. This caused multiple failures in cookbooks that expected that functionality from the v4.0.0 cookbook. The mechanics of notifications changed with chef 16 (15.8 really), but the notification features of v4.0.0 were not replicated with v8.4.0 with chef 16 functionality.
  • /etc/profile.d/java.sh is appended to, instead of being overwritten. Subsequent chef-client runs that install a new version of java will append a line. This seems not terribly impactful due to the fact that the last JAVA_HOME that is exported will take affect. However, if a java version is installed, and then a need arises to revert the upgrade then it causes issues since the JAVA_HOME line is added with the append_if_no_line resource. Which will not append the correct JAVA_HOME since the line already exists in the file. This will cause the bad version to be sourced as JAVA_HOME.
  • There are a large number of default attributes from v4.0.0 that are not included in v8.4.0. Any cookbook that depends on any of them will fail. Specifically we experienced issues with node['java']['jdk_version'].
  • I may be forgetting some. I will update if I remember any additional issues.

🥞 Cookbook version

We were upgrading from v4.0.0 to v8.4.0.

👩‍🍳 Chef-Infra Version

17:36:12 $ chef-client -version
Chef Infra Client: 16.6.14

🎩 Platform details

user@host:~ ( host )
17:36:38 $ uname -a
Linux host3.10.0-957.21.3.el7.x86_64 #1 SMP Mon Jun 17 15:42:47 PDT 2019 x86_64 x86_64 x86_64 GNU/Linux
user@host:~ ( host )
17:36:44 $ cat /etc/oracle-release
Oracle Linux Server release 7.9

Steps To Reproduce

A lot of steps involved in uplifting nodes from chef 14 to 16. The relevant, high level changes are:

  • create a new java recipe that is a thin wrapper around the adoptopenjdk_install resource to install java, since there is no longer a recipe included in v8.4.0. The recipe for the thin wrapper was initially, naively implemented as:
adoptopenjdk_install '8' do
  flavor = 'adoptopenjdk'

  variant = node['java'][flavor]['variant']
  version = node['java'][flavor]['version']
  arch = node['java'][flavor]['arch']

  tarball_url = node['java'][flavor][version][arch][variant]['url']
  tarball_checksum = node['java'][flavor][version][arch][variant]['checksum']

  variant variant
  url tarball_url
  checksum tarball_checksum
end
  • Replace the old 'recipe[java]' with the thin wrapper recipe listed above.
  • Upgrade a node with chef 14 to use chef 16
  • Run chef-client to re-deploy with chef 16.
  • ???
  • Experience fun

🚓 Expected behavior

The v8.4.0 recipe should account for the workflow of upgrading from older versions of the java cookbook. This includes clean-up of artifacts from previous installations that would cause issues, allow java version upgrades similar to previous versions, and providing like for like features such as notifications when the jdk version changes.

➕ Additional context

Add any other context about the problem here. e.g. related issues or existing pull requests.

@ramereth
Copy link
Contributor

I don't think its within the scope of this (or any of our cookbooks) to cleanup artifacts between versions. Ideally, this should be done within a wrapper cookbook by a user. If we handled that for every version of our cookbook, when would we be OK to remove code that would clean up older versions of the cookbook? We're a small volunteer team so we try to minimize the amount of code we maintain.

However, we have in some cases at least documented changes between major versions in an UPGRADING.md doc to help users (see an example in the apache2 cookbook). Perhaps this might be a good place to put such document for other users encountering the issues you're running into.

Would you be interested in creating a PR for such a doc?

@ramereth ramereth added Documentation Improvements or additions to documentation Waiting on Contributor Awaiting on the person who raised this to update labels Jan 29, 2021
@brailsmt
Copy link
Author

brailsmt commented Jan 29, 2021

From my perspective, as a consumer of the community java cookbook, these issues have made the experience of using the cookbook painful. I think it is a reasonable expectation that when I consume a cookbook, upgrades between versions are facilitated, in some way, by the cookbook itself. At the very least, I would expect documentation and a failure mechanism that looks for old artifacts and fails fast, rather than succeed in a state that is potentially harmful. In our case, we were surprised several times that a successful chef-client run did not result in the correct installation of java using the java install resources provided in sous-chef/java. This was especially surprising since we were upgrading from a previous version. The implementation of the cookbook changed significantly between v4.0.0 and v8.4.0. The effort to troubleshoot these issues was significant, and I think it is worthwhile addressing them here to prevent all consumers from encountering the same issues in practice. As our efforts to uplift to chef 16 wraps up, I will see if I can create some PRs based on our work that we can contribute back to the community and which will address the issues we found.

@jakauppila
Copy link
Contributor

While there is an expectation of compatibility within a major version of a cookbook the act of incrementing that major version is to indicate a breaking change and it is the responsibility of the consumer to ensure compatibility in its usage.

So to echo @ramereth, ideally an UPGRADING.md would be maintained to highlight what those breaking changes are and how to address them as you progress through the major versions. Expecting that the codebase handles the configuration of all previous versions is unrealistic.

@brailsmt
Copy link
Author

I apologise for the delay in responding, I've been quite busy with uplifting all our cookbooks and environments to chef 16. I feel that it is worthwhile to ensure that a consumer upgrading versions does not result in inconsistent states on the node, even if it is a major version upgrade. However, if the maintainers have decided that the cookbook will take a hands off approach to consumers upgrading and will only provide documentation, then I'm not sure there is a point to submitting a pull request with proposed changes. If you are interested, let me know and I will see what I can do to contribute the work we did back to the community so others may benefit.

@damacus
Copy link
Member

damacus commented Jun 5, 2021

@brailsmt the introduction of an UPGRADING.md would be really helpful and greatly received.

In your case you went up 4 major versions, which has it's own problems. If you're able pin down changes to needed to make for each major and document that in the UPGRADING doc that would be great!

Closing as I'm now considering this resolved.

@damacus damacus closed this as completed Jun 5, 2021
@brailsmt
Copy link
Author

brailsmt commented Jun 6, 2021

I view this as a much larger issue than just documentation. As the maintainers don't share the same view, I don't feel that contributions to resolve the issues would be well received, so I have moved on to other issues/tasks.

@damacus
Copy link
Member

damacus commented Jun 7, 2021

@brailsmt I think I'm missing something here, what else apart from docs do you think we're missing here? The feedback will most likely help us guide how we upgrade cookbooks in the future 👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation Waiting on Contributor Awaiting on the person who raised this to update
Projects
None yet
Development

No branches or pull requests

4 participants