Skip to content

Changes needed for agent-ng #42

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.

Sign up for GitHub

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

Closed
brendanbenson wants to merge 1 commit into from
Closed

Changes needed for agent-ng #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions lib/java_buildpack/container/tomcat/tomcat_insight_support.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ def detect
# @macro base_component_compile
def compile
link_to(container_libs_directory.children, tomcat_lib) if container_libs_directory.exist?
link_to(extra_applications_directory.children, tomcat_webapps) if extra_applications_directory.exist?
end

# @macro base_component_release
Expand All @@ -46,11 +45,6 @@ def release
def container_libs_directory
@droplet.root + '.spring-insight/container-libs'
end

def extra_applications_directory
@droplet.root + '.spring-insight/extra-applications'
end

end

end
Expand Down
45 changes: 17 additions & 28 deletions lib/java_buildpack/framework/spring_insight.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class SpringInsight < JavaBuildpack::Component::BaseComponent
# @param [Hash] context a collection of utilities used the component
def initialize(context)
super(context)
@version, @uri = supports? ? find_insight_agent : [nil, nil]
@version, @uri, @agent_id, @agent_pass = supports? ? find_insight_agent : [nil, nil, nil, nil]
end

# @macro base_component_detect
Expand Down Expand Up @@ -79,8 +79,8 @@ def add_agent_configuration
.add_system_property('agent.http.host', URI(@uri).host)
.add_system_property('agent.http.port', 80)
.add_system_property('agent.http.context.path', 'insight')
.add_system_property('agent.http.username', 'spring')
.add_system_property('agent.http.password', 'insight')
.add_system_property('agent.http.username', @agent_id)
.add_system_property('agent.http.password', @agent_pass)
.add_system_property('agent.http.send.json', false)
.add_system_property('agent.http.use.proxy', false)
end
Expand Down Expand Up @@ -111,21 +111,16 @@ def install_insight(agent_dir)

init_container_libs root
init_insight_cloudfoundry_agent_plugin root
init_extra_applications root
init_insight root
init_insight_analyzer root
init_insight_agent_plugins root
init_weaver root
end

def init_container_libs(root)
move container_libs_directory,
root + 'agents/common/insight-bootstrap-generic-*.jar',
root + 'agents/tomcat/7/lib/insight-bootstrap-tomcat-common-*.jar'
end

def init_extra_applications(root)
move extra_applications_directory,
root + 'insight-agent'
root + 'agents/tomcat/7/lib/insight-bootstrap-tomcat-common-*.jar',

@nebhale nebhale Mar 19, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems that this only takes into account the possibility of Tomcat 7. We support Tomcat 6 (although it's fading into the past, so I'm OK not supporting it here), but we also are starting to support Tomcat 8. Will the binaries in Tomcat 7 work in Tomcat 8?

@nebhale nebhale Mar 19, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't consider this something that necessarily needs to change (it's roughly the same code as before), just want to get an idea of where our limitations are.

@nebhale nebhale Mar 20, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@brendanbenson can you please comment on this one?

@jcpivotallabs jcpivotallabs Mar 24, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Speaking on behalf of @brendanbenson , we only support Tomcat 7.

@nebhale nebhale Mar 24, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jcpivotallabs Any idea what happens if the JARs end up in Tomcat 8? Is it innocuous or does it cause a failure (in which case we should probably ensure that the JARs aren't copied for Tomcat 8)?

@jcpivotallabs jcpivotallabs Mar 24, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The Insight agent doesn't support Tomcat 8. We can do some testing to see if deploying the Insight agent for Tomcat 7 to Tomcat 8 crashes the Java app.

Is there a way in the buildpack code to find out what version of Tomcat we're deploying to? If so, we could conditionally check that when copying the JAR file.

@jcpivotallabs jcpivotallabs Mar 24, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Initial test results:

$ java -jar insight-agent.jar --install --server_type tomcat --server_version 8.0.3
Error: Operation failed: No tomcat version 8.0.3 support

I think we'll need to exclude this JAR from Tomcat 8.

root + 'agents/tomcat/7/lib/insight-agent-*.jar'
end

def init_insight(root)
Expand All @@ -134,8 +129,8 @@ def init_insight(root)
root + 'insight/conf'
end

def init_insight_analyzer(root)
move insight_analyzer_directory + 'WEB-INF/lib',
def init_insight_agent_plugins(root)
move insight_directory + 'agent-plugins',
root + 'transport/http/insight-agent-http-*.jar',
root + 'cloudfoundry/insight-agent-cloudfoundry-*.jar'
end
Expand All @@ -147,27 +142,21 @@ def init_insight_cloudfoundry_agent_plugin(root)

def init_weaver(root)
move weaver_directory,
root + 'agents/common/insight-weaver-*.jar'
root + 'cloudfoundry/insight-weaver-*.jar'
end

def container_libs_directory
@droplet.root + '.spring-insight/container-libs'
end

def extra_applications_directory
@droplet.root + '.spring-insight/extra-applications'
end

def find_insight_agent
service = @application.services.find_service FILTER
version = service['label'].match(/(.*)-(.*)/)[2]
uri = service['credentials']['dashboard_url']

return version, uri # rubocop:disable RedundantReturn
end

def insight_analyzer_directory
extra_applications_directory + 'insight-agent'
service = @application.services.find_service FILTER
version = service['label'].match(/(.*)-(.*)/)[2]
credentials = service['credentials']
uri = credentials['dashboard_url']
id = credentials['agent_username']

@nebhale nebhale Mar 19, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The matching functionality in the Tomcat container needs to be removed.

@nebhale nebhale Mar 20, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now taken care of.

pass = credentials['agent_password']
return version, uri, id, pass # rubocop:disable RedundantReturn
end

def insight_directory
Expand All @@ -187,7 +176,7 @@ def move(destination, *globs)
end

def supports?
@application.services.one_service? FILTER, 'dashboard_url'
@application.services.one_service? FILTER, 'dashboard_url', 'agent_username', 'agent_password'
end

def uber_agent_zip(location)
Expand Down
Binary file modified spec/fixtures/stub-insight-agent.jar
View file
Open in desktop
Binary file not shown.
22 changes: 0 additions & 22 deletions spec/java_buildpack/container/tomcat/tomcat_insight_support_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,28 +32,6 @@
component.release
end

context do
let(:extra_applications_dir) { app_dir + '.spring-insight/extra-applications' }

before do
FileUtils.mkdir_p extra_applications_dir
FileUtils.cp_r 'spec/fixtures/framework_spring_insight', extra_applications_dir
end

it 'should link extra applications to the applications directory' do

component.compile

webapps_dir = sandbox + 'webapps'

insight_test_dir = webapps_dir + 'framework_spring_insight'
expect(insight_test_dir).to exist
expect(insight_test_dir).to be_symlink
expect(insight_test_dir.readlink).to eq((extra_applications_dir + 'framework_spring_insight')
.relative_path_from(webapps_dir))
end
end

context do
let(:container_libs_dir) { app_dir + '.spring-insight/container-libs' }

Expand Down
18 changes: 7 additions & 11 deletions spec/java_buildpack/framework/spring_insight_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
context do

before do
allow(services).to receive(:one_service?).with(/insight/, 'dashboard_url').and_return(true)
allow(services).to receive(:one_service?).with(/insight/, 'dashboard_url', 'agent_username', 'agent_password').and_return(true)
allow(services).to receive(:find_service).and_return('label' => 'insight-1.0',
'credentials' => { 'dashboard_url' => 'test-uri' })
'credentials' => { 'dashboard_url' => 'test-uri', 'agent_password' => 'foo', 'agent_username' => 'bar' })
allow(application_cache).to receive(:get).with('test-uri/services/config/agent-download')
.and_yield(Pathname.new('spec/fixtures/stub-insight-agent.jar').open)
end
Expand All @@ -44,17 +44,11 @@
component.compile

container_libs_dir = app_dir + '.spring-insight/container-libs'
extra_applications_dir = app_dir + '.spring-insight/extra-applications'

expect(sandbox + 'weaver/insight-weaver-1.2.4-CI-SNAPSHOT.jar').to exist
expect(container_libs_dir + 'insight-bootstrap-generic-1.2.3-CI-SNAPSHOT.jar').to exist
expect(container_libs_dir + 'insight-bootstrap-tomcat-common-1.2.5-CI-SNAPSHOT.jar').to exist
expect(sandbox + 'weaver/insight-weaver-cf-2.0.0-CI-SNAPSHOT.jar').to exist
expect(container_libs_dir + 'insight-bootstrap-generic-2.0.0-CI-SNAPSHOT.jar').to exist
expect(container_libs_dir + 'insight-bootstrap-tomcat-common-2.0.0-CI-SNAPSHOT.jar').to exist
expect(sandbox + 'insight/conf/insight.properties').to exist
expect(sandbox + 'insight/collection-plugins/test-collection-plugins').to exist
expect(extra_applications_dir + 'insight-agent').to exist
expect(extra_applications_dir + 'insight-agent/WEB-INF/lib/insight-agent-http-1.9.3-CI-SNAPSHOT.jar').to exist
expect(extra_applications_dir + 'insight-agent/WEB-INF/lib/insight-agent-cloudfoundry-1.2.3.jar').to exist
expect(container_libs_dir + 'cloudfoundry-runtime-1.2.3.jar').to exist
end

it 'should update JAVA_OPTS',
Expand All @@ -67,6 +61,8 @@
expect(java_opts).to include('-Dinsight.logs=$PWD/.java-buildpack/spring_insight/insight/logs')
expect(java_opts).to include('-Daspectj.overweaving=true')
expect(java_opts).to include('-Dorg.aspectj.tracing.factory=default')
expect(java_opts).to include('-Dagent.http.username=bar')
expect(java_opts).to include('-Dagent.http.password=foo')
end
end

Expand Down